use PurePath if path not provided to preserve symlinks#20249
use PurePath if path not provided to preserve symlinks#20249noobCodec wants to merge 8 commits intopython:masterfrom
Conversation
for more information, see https://pre-commit.ci
|
OK, I've tested it and this does, indeed, fix the problem on Windows! This is the old pr, for reference: #20161 |
| this_file_dir = os.path.dirname(PurePath(__file__)) | ||
| PREFIX = os.path.dirname(os.path.dirname(this_file_dir)) |
There was a problem hiding this comment.
To be clear, I was thinking that we can replace os.path.dirname with the relevant pathlib operation (this_file_dir = (Path(__file__) / '..').resolve()?)
There was a problem hiding this comment.
is there an advantage of doing this over just this_file_dir = (Path(__file__)).resolve().parent ?
There was a problem hiding this comment.
No I just don't know pathlib APIs...
for more information, see https://pre-commit.ci
A5rocks
left a comment
A5rocks
left a comment
There was a problem hiding this comment.
This looks better to me! A couple nitpicks but I like this more than what was here before.
@wyattscarpenter could you make sure it still fixes #19956?
FYI I'm unable to resolve comments, so feel free to.
| else: | ||
| this_file_dir = os.path.dirname(os.path.realpath(__file__)) | ||
| this_file_dir = (Path(__file__)).resolve().parent | ||
| PREFIX = os.path.dirname(os.path.dirname(this_file_dir)) |
There was a problem hiding this comment.
I think this can also use .parent. Maybe you'll need to do str(...) to make sure it's a string path at the end, though...
On the other hand, maybe it would be nice to replace all os.path with pathlib.Path native things in this file? Up to you!
|
I wouldn't merge this one yet looks like it breaks on certain windows machines with this one |
|
@wyattscarpenter this should be ready for re-test finally |
|
Yep, this new 1ef94dc revision still seems to fix the problem! |
|
I'd love it if this could be merged soon. |
4 participants
Fixes #19956. Updated this_file_dir in conf.py to use PurePath instead of realpath respecting symlink directories in tests.
This is an update of an earlier PR. Decided to use PurePath from UrlLib instead after a commend from A5rocks.
@wyattscarpenter Please retest.