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

[Brett Cannon]

PEP 519 updated: https://hg.python.org/peps/rev/92feff129ee4

On Wed, 15 Jun 2016 at 11:44 Brett Cannon <brett at python.org> wrote:

> On Wed, 15 Jun 2016 at 11:39 Guido van Rossum <guido at python.org> wrote:
>
>> OK, so let's add a check on the return of __fspath__() and keep the check
>> on path-like or string/bytes.
>>
>
> I'll update the PEP.
>
> Ethan, do you want to leave a note on the os.fspath() issue to update the
> code and go through where we've used os.fspath() to see where we can cut
> out redundant type checks?
>
>
>> --Guido (mobile)
>> On Jun 15, 2016 11:29 AM, "Nick Coghlan" <ncoghlan at gmail.com> wrote:
>>
>>> On 15 June 2016 at 10:59, Brett Cannon <brett at python.org> wrote:
>>> >
>>> >
>>> > 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).
>>>
>>> I'd like a strong assertion that the return value of os.fspath() is a
>>> plausible filesystem path representation (so either bytes or str), and
>>> *not* some other kind of object that can also be used for accessing
>>> the filesystem (like a file descriptor or an IO stream)
>>>
>>> >> 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)
>>>
>>> Note that the LZMA case Ethan cites is one where the code accepts
>>> either an already opened file-like object *or* a path-like object, and
>>> does different things based on which it receives.
>>>
>>> In that scenario, rather than introducing an unconditional "filename =
>>> os.fspath(filename)" before the current logic, it makes more sense to
>>> me to change the current logic to use the new protocol check rather
>>> than a strict typecheck on str/bytes:
>>>
>>>     if isinstance(filename, os.PathLike): # Changed line
>>>         filename = os.fspath(filename)    # New line
>>>         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 path-like or file-like object"
>>>               )
>>>
>>> I *don't* think it makes sense to weaken the guarantees on os.fspath
>>> to let it propagate non-path-like objects.
>>>
>>> Cheers,
>>> Nick.
>>>
>>> --
>>> Nick Coghlan   |   ncoghlan at gmail.com   |   Brisbane, Australia
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20160615/6476a503/attachment-0001.html>