You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
os.path.exists() takes bool as argument and returns True #82626
os.path.exists() accepts either True or False as argument and returns True.
Reproducible on Windows, e. g., in jupyter notebook or in Eclipse, although not in IDLE, which returns False, as expected.
Note that the underlying stat call supports file descriptors, which are non-negative integers. This is a supported and tested capability for genericpath.exists (see GenericTest.test_exists_fd in Lib/test/test_genericpath.py).
False and True are integers with the values 0 and 1:
That can be useful, but there may be cases where we don't want to conflate bools and integers. IMO, a bool should not be supported as a file descriptor. It's likely a bug that should be caught early instead of meaninglessly propagated.
A high-level solution would check for bool instances in genericpath.exists. A low-level solution, to make this policy consistent in general, would be to modify _fd_converter in Modules/posixmodule.c to disallow bool instances.
I think that would already be the case if genericpath.exists didn't have to support file descriptors. It's documented that the path argument "refers to an existing path or an open file descriptor".
Modifying _fd_converter would provide consistent behavior for all calls that ultimately use argument clinic's path_t(allow_fd=True) and/or dir_fd. Serhiy's suggestion to raise a warning sounds good.
Here are some examples that currently pass silently in Linux:
Probably os.fstat and other "f" functions (e.g. fstatvfs, fchdir, fchown, fchmod, ftruncate, fdatasync, fsync, and fpathconf) should also raise a warning when passed a bool. For example, the following would raise a warning instead of passing silently:
>>> os.fstat(False).st_size
0
These cases could be addressed by consistently using an argument clinic type. Some of them already us the fildes type (e.g. fchdir, fsync, fdatasync). However, fildes_converter calls PyObject_AsFileDescriptor, which also supports objects with a fileno() method. That's documented behavior for PyObject_AsFileDescriptor, but nothing in the documentation of fchdir, fsync, and fdatasync suggests to me that they support objects with a fileno() method:
>>> os.fchdir(sys.stdin)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
NotADirectoryError: [Errno 20] Not a directory
If not for this behavior, we could simply change all of the "f" functions to use the fildes type.
PyObject_AsFileDescriptor is used in various other places as well, such as the select module. In the latter case, supporting objects with a fileno() method is clearly documented.
Also consider including open() and os.fdopen by modifying _io_open_impl in Modules/_io/_iomodule.c. For example:
Activity
Nika commentedon Oct 11, 2019
os.path.exists() accepts either True or False as argument and returns True.
Reproducible on Windows, e. g., in jupyter notebook or in Eclipse, although not in IDLE, which returns False, as expected.
rmariano commentedon Oct 11, 2019
Hi, I would like to take a look at this issue. Could someone please assign it to me? Thanks
eryksun commentedon Oct 11, 2019
Note that the underlying stat call supports file descriptors, which are non-negative integers. This is a supported and tested capability for genericpath.exists (see GenericTest.test_exists_fd in Lib/test/test_genericpath.py).
False and True are integers with the values 0 and 1:
>>> issubclass(bool, int) True >>> False + 0 0 >>> True + 0 1That can be useful, but there may be cases where we don't want to conflate bools and integers. IMO, a bool should not be supported as a file descriptor. It's likely a bug that should be caught early instead of meaninglessly propagated.
A high-level solution would check for bool instances in genericpath.exists. A low-level solution, to make this policy consistent in general, would be to modify _fd_converter in Modules/posixmodule.c to disallow bool instances.
serhiy-storchaka commentedon Oct 11, 2019
Eryk, I think testing for bool in _fd_converter and issuing a warning is a good idea. Do you want to create a PR?
vstinner commentedon Oct 11, 2019
Maybe os.fspath() can be used:
>>> os.fspath(True) Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: expected str, bytes or os.PathLike object, not booltaleinat commentedon Oct 11, 2019
Mariano Anaya, you're welcome to work on this and create a PR.
There's no need to have this issue assigned to you: feel free to just begin working. Anyone reading this will know that you intended to work on this.
(FYI, we only assign issues to core developers, and assigning means something slightly different than "who is working on this.")
eryksun commentedon Oct 12, 2019
I think that would already be the case if genericpath.exists didn't have to support file descriptors. It's documented that the path argument "refers to an existing path or an open file descriptor".
Modifying _fd_converter would provide consistent behavior for all calls that ultimately use argument clinic's path_t(allow_fd=True) and/or dir_fd. Serhiy's suggestion to raise a warning sounds good.
Here are some examples that currently pass silently in Linux:
Probably os.fstat and other "f" functions (e.g. fstatvfs, fchdir, fchown, fchmod, ftruncate, fdatasync, fsync, and fpathconf) should also raise a warning when passed a bool. For example, the following would raise a warning instead of passing silently:
>>> os.fstat(False).st_size 0These cases could be addressed by consistently using an argument clinic type. Some of them already us the fildes type (e.g. fchdir, fsync, fdatasync). However, fildes_converter calls PyObject_AsFileDescriptor, which also supports objects with a fileno() method. That's documented behavior for PyObject_AsFileDescriptor, but nothing in the documentation of fchdir, fsync, and fdatasync suggests to me that they support objects with a fileno() method:
>>> os.fchdir(sys.stdin) Traceback (most recent call last): File "<stdin>", line 1, in <module> NotADirectoryError: [Errno 20] Not a directoryIf not for this behavior, we could simply change all of the "f" functions to use the fildes type.
PyObject_AsFileDescriptor is used in various other places as well, such as the select module. In the latter case, supporting objects with a fileno() method is clearly documented.
Also consider including open() and os.fdopen by modifying _io_open_impl in Modules/_io/_iomodule.c. For example:
>>> open(False, closefd=False) <_io.TextIOWrapper name=False mode='r' encoding='UTF-8'>Currently it calls PyNumber_Check(file). If true, a warning could be raised if PyBool_Check(file) is also true.
serhiy-storchaka commentedon Sep 22, 2020
It is a large change, so I take it.
pythongh-82626: Emit a warning when bool is used as a file descriptor
gh-82626: Emit a warning when bool is used as a file descriptor (GH-1…
pythongh-82626: Emit a warning when bool is used as a file descriptor (…
pythongh-82626: Schedule removal of bool used as file descriptor
gh-82626: Schedule removal of bool used as file descriptor (#145469)
pythongh-82626: Schedule removal of bool used as file descriptor (pyt…