On Wed, Jun 15, 2016 at 9:29 PM, 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)
I agree, so I'm -0.5 on passing through any object (at least by default). >>> 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 You are making one of my earlier points here, thanks ;). The point is that the name PathLike sounds like it would mean anything path-like, except that os.PathLike does not include str and bytes. And I still think the naming should be a little different. So that would be (os.Pathlike, str, bytes) instead of just os.PathLike. > 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/k7hoven%40gmail.com -- + Koos Zevenhoven + http://twitter.com/k7hoven + _______________________________________________ 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
