Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os.path.exists.__name__ is different on Windows since #101196 #115012

Closed
digitalresistor opened this issue Feb 4, 2024 · 3 comments
Closed

os.path.exists.__name__ is different on Windows since #101196 #115012

digitalresistor opened this issue Feb 4, 2024 · 3 comments
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes OS-windows type-bug An unexpected behavior, bug, or error

Comments

@digitalresistor
Copy link

digitalresistor commented Feb 4, 2024

Bug report

Bug description:

On macOS/Linux and Windows (before Python 3.12):

>>> import os
>>> os.path.exists.__name__
'exists'

Since Python 3.12 on Windows:

>>> import os
>>> os.path.exists.__name__
'_path_exists'

This was introduced in #101196 where optimized calls were made available on Windows.

Now, I will be honest that relying on the __name__ for os.path.exists is not great, but unfortunately I found this when running a test suite where we were relying on the __name__ being exists, which has been the case since at least Python 2.6 when I first took over maintenance of the project.

CPython versions tested on:

3.8, 3.9, 3.10, 3.11, 3.12

Operating systems tested on:

Linux, macOS, Windows

@digitalresistor digitalresistor added the type-bug An unexpected behavior, bug, or error label Feb 4, 2024
@AlexWaygood AlexWaygood added OS-windows 3.12 bugs and security fixes 3.13 bugs and security fixes labels Feb 4, 2024
@zooba
Copy link
Member

zooba commented Feb 5, 2024

What was the test checking for?

I don't think we can easily change the name, and many os.path functions names will vary based on platform and API availability (e.g. abspath).

Arguably it's more useful for the __name__ to reflect the actual implementation being used than to be hiding it.

@digitalresistor
Copy link
Author

@zooba checking that os.path could be imported and that it had an attribute named exists.__name__. It was used in the Waitress test suite to validate that the package loading works correctly, and instead of using something within the package itself to attempt loading os.path was used instead.

It was added before my time.

I don't think it was a good test, but I changed it from hardcoding exists to os.path.exists.__name__ and the tests now pass again, so it's no big deal, but it worked for many many years and changing the __name__ and it not matching the function name felt weird. That being said, if you want it to reflect the actual implementation used, I would argue that it should have information on where it is coming from, _path_exists IMHO is not a great name either.

@zooba
Copy link
Member

zooba commented Feb 5, 2024

_path_exists IMHO is not a great name either.

If you add __module__ then you get nt._path_exists rather than genericpath.exists, so it's about the same level of information (the two properties should be used together - there's some discussion about a __fullname__ but it'd just be these two added together). The _path_ prefix for native implementations that go into the ntpath/posixpath modules is standard.

Recognising the test relied on a poor assumption and fixing it is probably best here. We wouldn't change 3.12 anyway, and a sys.version_info[:2] == (3, 12) check might as well be >=.

@zooba zooba closed this as not planned Won't fix, can't repro, duplicate, stale Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes OS-windows type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants