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 — bpo-39659: Route calls from pathlib.Path to os.getcwd() via the path accessor by barneygale · Pull Request #18834 · python/cpython

Conversation

@barneygale

@barneygale barneygale commented Mar 8, 2020 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

(from the bug description:) Whereas most calls to os functions from pathlib.Path methods happen via pathlib._Accessor methods, retrieving the current working directory does not. This problem occurs when calling the pathlib.Path.cwd(), ~resolve() and ~absolute() methods.

https://bugs.python.org/issue39659

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@the-knights-who-say-ni

the-knights-who-say-ni commented Mar 8, 2020

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@barneygale

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@the-knights-who-say-ni the-knights-who-say-ni added the CLA not signed label Mar 8, 2020
@bedevere-bot bedevere-bot added the awaiting review label Mar 8, 2020
@barneygale

barneygale commented Mar 8, 2020

Copy link
Copy Markdown
Contributor Author

This change should have no functional effect beyond the addition of pathlib._NormalAccessor.getcwd(). which is an internal API anyway. The idea is to make the "accessor" abstraction less leaky and perhaps work our way towards a more extensible pathlib (see here)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@the-knights-who-say-ni the-knights-who-say-ni added CLA signed and removed CLA not signed labels Mar 9, 2020
pitrou
pitrou reviewed Apr 17, 2020
View reviewed changes
Comment thread Lib/pathlib.py Outdated

@pitrou pitrou Apr 17, 2020

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.

This is a quirky way to implement it. If the accessor becomes tied to the class rather than specific instances, perhaps it should become a class-level attribute?

Sorry, something went wrong.

Uh oh!

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

@barneygale barneygale May 29, 2020

Copy link
Copy Markdown
Contributor Author

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.

It's very similar to how home() is implemented immediately below.

Agree generally that the accessor should be a class attribute and not an instance attribute.

Sorry, something went wrong.

Uh oh!

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

@barneygale barneygale Jan 22, 2021

Copy link
Copy Markdown
Contributor Author

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've replaced cls() with cls in this method, and home() for consistency.

Sorry, something went wrong.

Uh oh!

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

@barneygale barneygale Jan 22, 2021

Copy link
Copy Markdown
Contributor Author

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.

Well, that didn't work at all! Looks like:

  • _accessor is set in _init(), at least until bpo-40038: pathlib: remove partial support for preserving accessor when modifying a path #19342 lands
  • _flavour is set in __new__().

Reverting to previous patch (the one you reviewed).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@barneygale
barneygale force-pushed the bpo-39659-pathlib-accessor-getcwd branch 3 times, most recently from cfc20dc to 05e73d0 Compare January 22, 2021 01:51
@zooba

zooba commented Apr 7, 2021

Copy link
Copy Markdown
Member

@barneygale This one looks like the conflict is meaningful? (And it's new because I merged one of your other changes, I think)
barneygale 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.
@barneygale
barneygale force-pushed the bpo-39659-pathlib-accessor-getcwd branch from 05e73d0 to 74abd98 Compare April 7, 2021 00:59
@barneygale

barneygale commented Apr 7, 2021

Copy link
Copy Markdown
Contributor Author

Thank you! Rebased :-)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@barneygale
bpo-39659: Route calls from pathlib.Path to os.getcwd() via the path …
74abd98 
…accessor
@zooba zooba added the skip news label Apr 7, 2021
@zooba
zooba merged commit b05440c into python:master Apr 7, 2021
@bedevere-bot bedevere-bot removed the awaiting review label Apr 7, 2021
@alexdelorenzo alexdelorenzo mentioned this pull request Apr 16, 2021
Python 3.10 alpha 7 introduces new changes to pathlib alexdelorenzo/aiopath#4
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pitrou pitrou pitrou left review comments

Uh oh!

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

Assignees

No one assigned

Labels

skip news

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.

5 participants

@barneygale @the-knights-who-say-ni @zooba @pitrou @bedevere-bot
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.