[Python-Dev] os.path.join failure mode

[Terry Reedy]

On 2/9/2013 8:31 AM, R. David Murray wrote:
> On Sat, 09 Feb 2013 09:59:13 +0000, Thomas Scrace <tom at scrace.org> wrote:
>> If a function (or other non-string object) is accidentally passed as an
>> argument to os.path.join() the result is an AttributeError:
>>
>>
>> In [3]: os.path.join(fn, "path")
>>> ---------------------------------------------------------------------------
>>> AttributeError                            Traceback (most recent call last)
>>> /home/tom/<ipython-input-3-44b097ceab04> in <module>()
>>> ----> 1 os.path.join(fn, "path")
>>> /usr/lib/python2.7/posixpath.pyc in join(a, *p)
>>>       66         if b.startswith('/'):
>>>       67             path = b
>>> ---> 68         elif path == '' or path.endswith('/'):
>>>       69             path +=  b
>>>       70         else:
>>> AttributeError: 'function' object has no attribute 'endswith'
>>
>> It's relatively easy to run into this if you mean to pass the return value
>> of a function (fn()) as the argument but accidentally forget to append
>> parens (()) to the callable, thus passing the function itself instead.
>>
>> Would it not be more helpful to raise a TypeError("Argument must be a
>> string") than the slightly more mysterious AttributeError?
>>
>> It's not the most difficult error in the world to figure out, I admit, but
>> a TypeError just seems like the more correct thing to do here.

I agree. Since the exception type is not documented and since no one 
should intentionally pass anything but strings, and therefore should not 
be writing

try:
   os.path.join(a,b)
except AttributeError:
   barf()

I think it would be acceptable to make a change in 3.4.

> We don't generally do that kind of type checking in Python.  Sometimes the
> error that results is obscure enough that adding an early check of some
> sort is worth it, but this one is very clear, so I don't see that an
> additional check would add much value here.

Changing AttributeError to TypeError only requires try-except, which is 
cheap if there is no error, not an early type check.

> The reason we avoid such type checks is that we prefer to operate via
> "duck typing", which means that if an object behaves like the expected
> input, it is accepted.

And catching the existing AttributeError allows work-alikes. I agree 
that an isinstance check is bad as well as unnecessary.

As it is, the core code for path, given above, is *already* wrapped in 
try: except TypeError, (to give a more friendly error message!, as 
TypeError is most like from a unicode-bytes mixing). So there would be 
no extra cost for correct calls and all that is needed is another except 
clause.

     except AttributeError as e:
         bad = e.args[0].split()[0]
         msg = "all components must be strings; one is a {}".format(bad)
         raise TypeError("all components must be strings")

Thomas, unless David or someone else shoots down this idea here, I say 
go ahead and open an enhancement issue and add terry.reedy as nosy.

-- 
Terry Jan Reedy