GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-109782: Ensure `os.path.isdir` has the same signature on all platforms by aminalaee · Pull Request #109790 · python/cpython

Conversation

@aminalaee

@aminalaee aminalaee commented Sep 23, 2023 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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: ...
  • Issue: Windows os.path.isdir has different signature #109782

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot mentioned this pull request Sep 23, 2023
Windows os.path.isdir has different signature #109782
Closed
@aminalaee
pythongh-109782 Update NT os.path.isdir signature
13d10df
@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
@bedevere-app bedevere-app Bot added the awaiting review label Sep 23, 2023
@AlexWaygood
Add news and tests
2e99591
@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
AlexWaygood
AlexWaygood approved these changes Sep 24, 2023
View reviewed changes

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

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.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting core review and removed awaiting review labels Sep 24, 2023
@AlexWaygood
AlexWaygood requested review from hauntsaninja and mdboom September 24, 2023 17:02
@zooba

zooba commented Sep 26, 2023

Copy link
Copy Markdown
Member

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).
AlexWaygood and aminalaee reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aminalaee

This comment was marked as outdated.

Sign in to view
@aminalaee
Remove test
9cc53cd
@zooba
zooba merged commit 7df8b16 into python:main Sep 28, 2023
@bedevere-app bedevere-app Bot removed the awaiting core review label Sep 28, 2023
@zooba

zooba commented Sep 28, 2023

Copy link
Copy Markdown
Member

Thanks!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood

AlexWaygood commented Sep 28, 2023

Copy link
Copy Markdown
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 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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
@aminalaee @csm10495
pythongh-109782: Ensure os.path.isdir has the same signature on all…
f7f79fe 
… platforms (pythonGH-109790)
@AlexWaygood AlexWaygood added the needs backport to 3.12 only security fixes label Oct 2, 2023
@miss-islington

miss-islington commented Oct 2, 2023

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington

miss-islington commented Oct 2, 2023

Copy link
Copy Markdown
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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington miss-islington assigned zooba Oct 2, 2023
@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
@aminalaee @AlexWaygood
pythongh-109782: Ensure os.path.isdir has the same signature on all…
1ca2a2f 
… platforms (pythonGH-109790)
@bedevere-app

bedevere-app Bot commented Oct 2, 2023

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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
@AlexWaygood @aminalaee
[3.12] gh-109782: Ensure os.path.isdir has the same signature on al…
da75bd6 
…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
@aminalaee @Glyphack
pythongh-109782: Ensure os.path.isdir has the same signature on all…
5a8bdec 
… platforms (pythonGH-109790)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes
@mdboom mdboom Awaiting requested review from mdboom
@hauntsaninja hauntsaninja Awaiting requested review from hauntsaninja

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@AlexWaygood AlexWaygood

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@aminalaee @zooba @AlexWaygood @miss-islington
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.