[Python-Dev] proposed os.fspath() change

[Brett Cannon]

On Wed, 15 Jun 2016 at 09:48 Guido van Rossum <guido at python.org> wrote:

> These are really two separate proposals.
>
> I'm okay with checking the return value of calling obj.__fspath__; that's
> an error in the object anyways, and it doesn't matter much whether we do
> this or not (though when approving the PEP I considered this and decided
> not to insert a check for this). But it doesn't affect your example, does
> it? I guess it's easier to raise now and change the API in the future to
> avoid raising in this case (if we find that raising is undesirable) than
> the other way around, so I'm +0 on this.
>

+0 from me as well. I know in some code in the stdlib that has been ported
which prior to adding support was explicitly checking for str/bytes this
will eliminate its own checking (obviously not a motivating factor as it's
pretty minor).


>
> The other proposal (passing anything that's not understood right through)
> is more interesting and your use case is somewhat compelling. Catching the
> exception coming out of os.fspath() would certainly be much messier. The
> question remaining is whether, when this behavior is not desired (e.g. when
> the caller of os.fspath() just wants a string that it can pass to open()),
> the condition of passing that's neither a string not supports __fspath__
> still produces an understandable error. I'm not sure that that's the case.
> E.g. open() accepts file descriptors in addition to paths, but I'm not sure
> that accepting an integer is a good idea in most cases -- it either gives a
> mystery "Bad file descriptor" error or starts reading/writing some random
> system file, which it then closes once the stream is closed.
>

The FD issue of magically passing through an int was also a concern when
Ethan brought this up in an issue on the tracker. My argument is that FDs
are not file paths and so shouldn't magically pass through if we're going
to type-check anything or claim os.fspath() only works with paths (FDs are
already open file objects). So in my view  either we go ahead and
type-check the return value of __fspath__() and thus restrict everything
coming out of os.fspath() to Union[str, bytes] or we don't type check
anything and be consistent that os.fspath() simply does is call
__fspath__() if present.

And just  because I'm thinking about it, I would special-case the FDs, not
os.PathLike (clearer why you care and faster as it skips the override of
__subclasshook__):

# Can be a single-line ternary operator if preferred.
if not isinstance(filename, int):
    filename = os.fspath(filename)


> On Wed, Jun 15, 2016 at 9:12 AM, Ethan Furman <ethan at stoneleaf.us> wrote:
>
>> I would like to make a change to os.fspath().
>>
>> Specifically, os.fspath() currently raises an exception if something
>> besides str, bytes, or os.PathLike is passed in, but makes no checks
>> if an os.PathLike object returns something besides a str or bytes.
>>
>> I would like to change that to the opposite: if a non-os.PathLike is
>> passed in, return it unchanged (so no change for str and bytes); if
>> an os.PathLike object returns something that is not a str nor bytes,
>> raise.
>>
>> An example of the difference in the lzma file:
>>
>> Current code (has not been upgraded to use os.fspath() yet)
>> -----------------------------------------------------------
>>
>>     if isinstance(filename, (str, bytes)):
>>         if "b" not in mode:
>>             mode += "b"
>>         self._fp = builtins.open(filename, mode)
>>         self._closefp = True
>>         self._mode = mode_code
>>     elif hasattr(filename, "read") or hasattr(filename, "write"):
>>         self._fp = filename
>>         self._mode = mode_code
>>     else:
>>         raise TypeError(
>>              "filename must be a str or bytes object, or a file"
>>               )
>>
>> Code change if using upgraded os.fspath() (placed before above stanza):
>>
>>     filename = os.fspath(filename)
>>
>> Code change with current os.fspath() (ditto):
>>
>>     if isinstance(filename, os.PathLike):
>>         filename = os.fspath(filename)
>>
>> My intention with the os.fspath() function was to minimize boiler-plate
>> code and make PathLike objects easy and painless to support; having to
>> discover if any given parameter is PathLike before calling os.fspath()
>> on it is, IMHO, just the opposite.
>>
>> There is also precedent for having a __dunder__ check the return type:
>>
>>     --> class Huh:
>>     ...   def __int__(self):
>>     ...     return 'string'
>>     ...   def __index__(self):
>>     ...     return b'bytestring'
>>     ...   def __bool__(self):
>>     ...     return 'true-ish'
>>     ...
>>     --> h = Huh()
>>
>>     --> int(h)
>>     Traceback (most recent call last):
>>       File "<stdin>", line 1, in <module>
>>     TypeError: __int__ returned non-int (type str)
>>
>>     --> ''[h]
>>     Traceback (most recent call last):
>>       File "<stdin>", line 1, in <module>
>>     TypeError: __index__ returned non-int (type bytes)
>>
>>     --> bool(h)
>>     Traceback (most recent call last):
>>       File "<stdin>", line 1, in <module>
>>     TypeError: __bool__ should return bool, returned str
>>
>> Arguments in favor or against?
>>
>> --
>> ~Ethan~
>> _______________________________________________
>> Python-Dev mailing list
>> Python-Dev at python.org
>> https://mail.python.org/mailman/listinfo/python-dev
>>
> Unsubscribe:
>> https://mail.python.org/mailman/options/python-dev/guido%40python.org
>>
>
>
>
> --
> --Guido van Rossum (python.org/~guido)
> _______________________________________________
> Python-Dev mailing list
> Python-Dev at python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/brett%40python.org
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20160615/65314c9e/attachment-0001.html>