Skip to content

GH-92737: Corrected posixpath behaviour to not assume '//' is equivalent to '/'. #103798

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

GH-92737: Corrected posixpath behaviour to not assume '//' is equivalent to '/'. #103798

wants to merge 10 commits into from
+22 −3

Conversation

achhina
Copy link
Contributor

@achhina achhina commented Apr 24, 2023

Fixes GH-92737 - added roots equivalent check to posixpath.relpath similar to ntpath.relpath.

  • Added new test cases for posixpath.relpath
  • Tests pass locally
  • Added News Entry

@bedevere-bot bedevere-bot mentioned this pull request Apr 24, 2023
Open
@achhina
Copy link
Contributor Author

achhina commented Apr 24, 2023

@barneygale would you be able to review this when you get a chance? I wasn't sure how disruptive this would be for users who were relying on the previous implementation, so I haven't added in any warnings/deprecation, but happy to do so if desired.

barneygale
barneygale requested changes Apr 29, 2023
View reviewed changes
@bedevere-bot
Copy link

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. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

achhina and others added 2 commits April 30, 2023 14:38
@achhina @barneygale
Update Lib/posixpath.py
a35bf4e 
Co-authored-by: Barney Gale <barney.gale@gmail.com>
@achhina @barneygale
Update Lib/posixpath.py
bc54f86 
Co-authored-by: Barney Gale <barney.gale@gmail.com>
@achhina
Copy link
Contributor Author

achhina commented Apr 30, 2023

I didn't expect the Spanish Inquisition

@bedevere-bot
Copy link

Nobody expects the Spanish Inquisition!

@barneygale: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from barneygale April 30, 2023 18:44
@achhina
Copy link
Contributor Author

achhina commented Aug 22, 2023

@barneygale would you be able to take a look at this when you get a chance?

@achhina
Copy link
Contributor Author

achhina commented Dec 3, 2023

Hi, friendly ping @barneygale, would you mind taking another look at this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@barneygale barneygale Awaiting requested review from barneygale

Assignees
No one assigned
Labels
awaiting change review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

os.path.relpath() assumes '//' root is equivalent to '/'
4 participants
@achhina @bedevere-bot @barneygale @ambv