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 — fix(types): get the os.PathLike type as correctly as possible by muggenhor · Pull Request #1227 · gitpython-developers/GitPython

Conversation

@muggenhor

@muggenhor muggenhor commented Apr 23, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

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.

All reactions
@muggenhor
fix(types): get the os.PathLike type as correctly as possible
c996f89
This should make our internal PathLike type compatible with Python <
3.6 and < 3.9.
@muggenhor

muggenhor commented Apr 23, 2021

Copy link
Copy Markdown
Contributor Author

Switched away from try-except to explicit version checks because that seems to be the recommended style python/mypy#698

All reactions

Sorry, something went wrong.

Uh oh!

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

@muggenhor muggenhor mentioned this pull request Apr 23, 2021
ci: check types with mypy #1226
Merged
@Byron
Byron merged commit 9f12c8c into gitpython-developers:main Apr 23, 2021
@muggenhor
muggenhor deleted the fix/pathlike-type branch April 23, 2021 13:20
@Byron

Byron commented Apr 23, 2021

Copy link
Copy Markdown
Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

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

@muggenhor

muggenhor commented Apr 23, 2021

Copy link
Copy Markdown
Contributor Author

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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

@muggenhor

muggenhor commented Apr 24, 2021

Copy link
Copy Markdown
Contributor Author

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?

All reactions

Sorry, something went wrong.

Uh oh!

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

@Byron

Byron commented Apr 24, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

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

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

Reviewers

No reviews

Uh oh!

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

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Python 3.5 failing with AttributeError: module 'os' has no attribute 'PathLike'

Uh oh!

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

2 participants

@muggenhor @Byron
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 01402520 — 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.