PEP 519 updated: https://hg.python.org/peps/rev/92feff129ee4
On Wed, 15 Jun 2016 at 11:44 Brett Cannon <br...@python.org> wrote: > On Wed, 15 Jun 2016 at 11:39 Guido van Rossum <gu...@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" <ncogh...@gmail.com> wrote: >> >>> On 15 June 2016 at 10:59, Brett Cannon <br...@python.org> wrote: >>> > >>> > >>> > On Wed, 15 Jun 2016 at 09:48 Guido van Rossum <gu...@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 | ncogh...@gmail.com | Brisbane, Australia >>> >>
_______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com