GeezerVison is showing github.com — fix(types): get the os.PathLike type as correctly as possible by muggenhor · Pull Request #1227 · gitpython-developers/GitPython
Conversation
muggenhor
commented
Apr 23, 2021
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
This should make our internal PathLike type compatible with Python < 3.6 and < 3.9.
Fixes #1219
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This should make our internal PathLike type compatible with Python <
3.6 and < 3.9.
|
Switched away from try-except to explicit version checks because that seems to be the recommended style python/mypy#698
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Thanks, very elegant!
I find it interesting that all PathLike types seem to use str in some way even though AFAIK these are to be decoded with some encoding from what the OS provides. As this encoding is unspecified or at least very dependent on knowledge about the OS and its settings, using a path based on str can fail with some sort of decoding error.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
FYI: this is the type annotation definition of os.PathLike. So what os.PathLike[str] means is: it has to be a type with an __fscache__(self) -> str method. So that's the method where any conversions should be performed.
|
Byron reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
I find it interesting that all PathLike types seem to use str in some way even though AFAIK these are to be decoded with some encoding from what the OS provides. As this encoding is unspecified or at least very dependent on knowledge about the OS and its settings, using a path based on str can fail with some sort of decoding error.
Hmm. Does that mean you think it should accept bytes in some locations too?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Byron
commented
Apr 24, 2021
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Hmm. Does that mean you think it should accept bytes in some locations too?
I would refrain from doing anything that reduces convenience or being idiomatic or that is breaking just to gain correctness.
But I think that as it stands, GitPython will work fine until it doesn't because one tries to handle a path with characters that don't decode with the pre-configured default encoding. Right now this is UTF-8, certainly a decent choice. But it won't always work either.
To me the whole encoding topic in GitPython is a lost cause as correctness could (to my mind) only be achieved if the API doesn't use strings (with encoding) to handle them.
And by the looks of it, it's idiomatic to do that, too:
# py 3.8
>>> type(os.listdir('.')[0])
<class 'str'>
Edit: On the bright side, I believe just today I had an epiphany which helps me to see a future for GitPython, leveraging it as a strength and not seeing it as a burden. So I hope this will change my mindset too when helping to maintain it in the midterm until such a future can be realized.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
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.