-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
bpo-33591: Fix CDLL to accept PathLike objects #7032
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
Conversation
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. When your account is ready, please add a comment in this pull request Thanks again to your contribution and we look forward to looking at it! |
7970e84
to
abe38c6
Compare
I signed the CLA (my bugtracker name is "mrh1997"). |
1c1642e
to
461bc92
Compare
Due to issue23606 (find msvct does not work any more) test_load did not work any more, as it used MSVCRT for testing DLL loading. Now it uses _ctypes_test library, which is explicitly generated for testing purposes.
CDLL accepts now not only paths of type 'str' but also of type PathLike (like Path()).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry this PR took so long to review. I dug it up from page 57 of the open PR list, but I think it's pretty close to ready.
I do have some comments:
- https://docs.python.org/3.10/library/ctypes.html#ctypes.CDLL should have a "versionchanged" notice saying that starting in version 3.11, PathLike objects are also supported. A similar example note is the one in https://docs.python.org/3.10/library/shutil.html#shutil.move for 3.9. The note should also be added to the other *DLL classes.
- I'd suggest rewording the NEWS entry as:
:class:`ctypes.CDLL`, :class:`ctypes.OleDLL`, :class:`ctypes.WinDLL`,
and :class:`ctypes.PyDLL` now accept :term:`path-like objects
<path-like object>` as their ``name`` argument. Patch by Robert Hoelzl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has merge conflicts now.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
@mrh1997 Could you sign the new CLA by clicking |
No longer has merge conflicts
Non-Linux tests failed with the following error:
I'm investigating it. |
Fixed; there were two missed lines that continued to use |
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
I have made the requested changes; please review again. |
CDLL (and WinDLL, OleDLL, ...) accept now not only str objects but also PathLike objects.
https://bugs.python.org/issue33591