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

Brett Cannon brett at python.org
Wed Jun 15 15:57:28 EDT 2016

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 = modecode elif hasattr(filename, "read") or hasattr(filename, "write"): self.fp = filename self.mode = modecode 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>

More information about the Python-Dev mailing list