Skip to content

gh-109782: Ensure os.path.isdir has the same signature on all platforms #109790

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

Merged
merged 3 commits into from
Sep 28, 2023

Conversation

aminalaee
Copy link
Contributor

@aminalaee aminalaee commented Sep 23, 2023

To fix the os.path.isdir signature for Windows to be the same as genericpath version.

# in genericpath.py
def isdir(s) -> bool: ...

# in optimized nt module
def isdir(path) -> bool: ...

@aminalaee aminalaee force-pushed the fix-nt-os-path-isdir-signature branch from 0628d63 to 13d10df Compare September 23, 2023 17:06
@aminalaee aminalaee marked this pull request as ready for review September 23, 2023 17:46
@AlexWaygood AlexWaygood changed the title gh-109782: Update NT os.path.isdir signature gh-109782: Ensure os.path.isdir has the same signature on all platforms Sep 24, 2023
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zooba
Copy link
Member

zooba commented Sep 26, 2023

I don't think we need a test for this. More likely we need a written policy somewhere about how named arguments are handled in accelerator functions (which will probably retrigger the discussion about named vs. position-only arguments again).

@aminalaee

This comment was marked as outdated.

@zooba zooba merged commit 7df8b16 into python:main Sep 28, 2023
@zooba
Copy link
Member

zooba commented Sep 28, 2023

Thanks!

@AlexWaygood
Copy link
Member

FYI I'm planning to backport this, but only for 3.12.1 (it's not important enough to be backported this close to the release date for 3.12.0)

@aminalaee aminalaee deleted the fix-nt-os-path-isdir-signature branch September 28, 2023 15:27
csm10495 pushed a commit to csm10495/cpython that referenced this pull request Sep 29, 2023
@AlexWaygood AlexWaygood added the needs backport to 3.12 only security fixes label Oct 2, 2023
@miss-islington
Copy link
Contributor

Thanks @aminalaee for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @aminalaee and @zooba, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 7df8b16d28d2418161cef49814b6aca9fb70788d 3.12

@AlexWaygood AlexWaygood assigned AlexWaygood and unassigned zooba Oct 2, 2023
AlexWaygood pushed a commit to AlexWaygood/cpython that referenced this pull request Oct 2, 2023
@bedevere-app
Copy link

bedevere-app bot commented Oct 2, 2023

GH-110233 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 2, 2023
AlexWaygood added a commit that referenced this pull request Oct 2, 2023
…l platforms (GH-109790) (#110233)

gh-109782: Ensure `os.path.isdir` has the same signature on all platforms (GH-109790)

Co-authored-by: Amin Alaee <[email protected]>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants