NotificationsYou must be signed in to change notification settings
Fork32.2k
Star67.7k

bpo-29688: document and test`pathlib.Path.absolute()`.#26153

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

brettcannon merged 9 commits intopython:mainfrombarneygale:bpo-29688-pathlib-absolute-docs-tests

Jan 28, 2022

Merged

bpo-29688: document and test`pathlib.Path.absolute()`.#26153

brettcannon merged 9 commits intopython:mainfrombarneygale:bpo-29688-pathlib-absolute-docs-tests

Jan 28, 2022

Conversation

Copy link

Contributor

barneygale commentedMay 15, 2021•
edited by bedevere-bot
Loading

This method is important and cannot be replaced withresolve()!resolve() will hit OS APIs to resolve symlinks/etc, and will fail on symlink loops, whereasabsolute() just prepends the working directory and can never raise an OSError.

Also improved the table showing correspondence withos.path methods. The gory details:

Path.resolve() isos.path.realpath() in strict mode, with a tweaked exception type for symlink loops (backwards compat)
Path.absolute() isos.path.abspath() butwithout normalizing the path. The pathlib behaviour is better, asabspath()'s normlisation changes the meaning of the path in the presence of symlinks!

Happy to expand the tests if anyone has any ideas what else to cover.

https://bugs.python.org/issue29688

bedevere-bot added the awaiting review label

May 15, 2021

the-knights-who-say-ni added the CLA signed label

May 15, 2021

Copy link

kalvdans commentedMay 16, 2021

Documentation is clear and the suggested changes looks sane to me. Will take another look after windows test failures are addressed.

Copy link

ContributorAuthor

barneygale commentedMay 16, 2021

Tested are fixed, thanks for taking a look.

kalvdans approved these changes

May 16, 2021

View reviewed changes

Copy link

kalvdans left a comment

Choose a reason for hiding this comment

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

Some small remarks.

Lib/test/test_pathlib.py OutdatedShow resolvedHide resolved

bedevere-bot added awaiting core review and removed awaiting review labels

May 16, 2021

Copy link

ContributorAuthor

barneygale commentedMay 16, 2021•
edited
Loading

Note that this can't be automatically backported, as the tests' patching ofgetcwd() is different in 3.9 and below.

barneygale mentioned this pull request

May 16, 2021

bpo-43012: removepathlib._Accessor#25701

Merged

kalvdans approved these changes

May 17, 2021

View reviewed changes

eamanu reviewed

May 18, 2021

View reviewed changes

Doc/library/pathlib.rst OutdatedShow resolvedHide resolved

brettcannon self-requested a review

June 14, 2021 19:15

ngnpope mentioned this pull request

Jun 22, 2021

Replaced os module with pathlib for pathsdjango/django#14553

Closed

Copy link

github-actionsbot commentedJul 15, 2021

This PR is stale because it has been open for 30 days with no activity.

github-actionsbot added the staleStale PR or inactive for long period of time. label

Jul 15, 2021

domdfcoding reviewed

Jul 15, 2021

View reviewed changes

Doc/library/pathlib.rst OutdatedShow resolvedHide resolved

brettcannon requested changes

Jul 16, 2021

View reviewed changes

Copy link

Member

brettcannon left a comment•
edited
Loading

Choose a reason for hiding this comment

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

One clarifying doc change as suggested by@domdfcoding. A news entry is also missing.

Doc/library/pathlib.rst OutdatedShow resolvedHide resolved

bedevere-bot removed the awaiting core review label

Jul 16, 2021

Copy link

bedevere-bot commentedJul 16, 2021

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 phraseI 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.

bedevere-bot added the awaiting changes label

Jul 16, 2021

github-actionsbot removed the staleStale PR or inactive for long period of time. label

Jul 17, 2021

alneberg mentioned this pull request

Oct 18, 2021

Abspath instead of resolveScilifelabDataCentre/dds_cli#175

Merged

brianhelba reviewed

Oct 25, 2021

View reviewed changes

Doc/library/pathlib.rst OutdatedShow resolvedHide resolved

Copy link

ContributorAuthor

barneygale commentedJan 1, 2022

Sorry for the long delay. Does this need a NEWS entry? It adds docs and tests but doesn't change the implementation ofPath.absolute()

barneygaleand others added6 commits

January 1, 2022 01:21

bpo-29688: document and testpathlib.Path.absolute().

06b0c74

Add tests for UNC paths.

b5ff1e6

Fix Windows tests (attempt 1)

8695087

assertEqualNormCase() -->assertEqual()

5539824

Update Doc/library/pathlib.rst

7cd4fff

Co-authored-by: Brett Cannon <brett@python.org>

Update Doc/library/pathlib.rst

64204fd

Co-authored-by: Brian Helba <brian.helba@kitware.com>

barneygale force-pushed thebpo-29688-pathlib-absolute-docs-tests branch from644703a to64204fdCompare

January 1, 2022 01:21

barneygale requested a review frombrettcannon

January 1, 2022 03:06

Add news entry.

b2bf3da

Copy link

Member

brettcannon commentedJan 7, 2022

Sorry for the long delay. Does this need a NEWS entry? It adds docs and tests but doesn't change the implementation ofPath.absolute()

It looks like you added a news entry anyway. 😄

brettcannon requested changes

Jan 8, 2022

View reviewed changes

Copy link

Member

brettcannon left a comment

Choose a reason for hiding this comment

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

Question about the docs and some very minor formatting things.

Lib/test/test_pathlib.py OutdatedShow resolvedHide resolved

Misc/NEWS.d/next/Library/2022-01-05-03-21-21.bpo-29688.W06bSH.rst OutdatedShow resolvedHide resolved

Doc/library/pathlib.rst OutdatedShow resolvedHide resolved

barneygaleand others added2 commits

January 8, 2022 20:37

Apply suggestions from code review

2a9ebd0

Co-authored-by: Brett Cannon <brett@python.org>

Remove example in docs showing that.. entries are retained.

51aa187

barneygale requested a review frombrettcannon

January 16, 2022 23:07

brettcannon added awaiting change review and removed awaiting changes labels

Jan 17, 2022

brettcannon approved these changes

Jan 28, 2022

View reviewed changes

Lib/test/test_pathlib.py

Comment on lines +2662 to +2670

		self.assertEqual(str(P('c:\\').absolute()), 'c:\\')
		self.assertEqual(str(P('c:\\a').absolute()), 'c:\\a')
		self.assertEqual(str(P('c:\\a\\b').absolute()), 'c:\\a\\b')

		# UNC absolute paths.
		share = '\\\\server\\share\\'
		self.assertEqual(str(P(share).absolute()), share)
		self.assertEqual(str(P(share + 'a').absolute()), share + 'a')
		self.assertEqual(str(P(share + 'a\\b').absolute()), share + 'a\\b')

Copy link

Member

brettcannonJan 28, 2022

Choose a reason for hiding this comment

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

Suggested change

	self.assertEqual(str(P('c:\\').absolute()),'c:\\')
	self.assertEqual(str(P('c:\\a').absolute()),'c:\\a')
	self.assertEqual(str(P('c:\\a\\b').absolute()),'c:\\a\\b')

	# UNC absolute paths.
	share='\\\\server\\share\\'
	self.assertEqual(str(P(share).absolute()),share)
	self.assertEqual(str(P(share+'a').absolute()),share+'a')
	self.assertEqual(str(P(share+'a\\b').absolute()),share+'a\\b')
	self.assertEqual(str(P(r'c:\').absolute()),r'c:\')
	self.assertEqual(str(P(r'c:\a').absolute()),r'c:\a')
	self.assertEqual(str(P(r'c:\a\b').absolute()),r'c:\a\b')

	# UNC absolute paths.
	share='\\\\server\\share\\'
	self.assertEqual(str(P(share).absolute()),share)
	self.assertEqual(str(P(share+'a').absolute()),share+'a')
	self.assertEqual(str(P(share+r'a\b').absolute()),share+r'a\b')