Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git.types.PathLike is partially unknown for Pylance/mypy #1473

Closed
madebylydia opened this issue Jul 26, 2022 · 4 comments · Fixed by #1474
Closed

git.types.PathLike is partially unknown for Pylance/mypy #1473

madebylydia opened this issue Jul 26, 2022 · 4 comments · Fixed by #1474
Labels
acknowledged c-typing Mypy causes warnings or errors when run help wanted

Comments

@madebylydia
Copy link
Contributor

Hello everyone,

I am using GitPython in my project and I use Pylance/mypy with the strict type checking settings. This setting is pretty important for my project's sanity.

As I've been using methods from GitPython that make uses of paths, I've been getting the following errors:
mypy raising an error for git.types.PathLike
As you can notice, what raises this error is PathLike[Unknown], defined in git.types.PathLike. (Source file & lines)

After further investigation, I've found out that this is because os.PathLike is considered as a generic class (Like typing.List, typing.Dict and so on)
Image showing PathLike being a generic class

The fix is fairly simple, as it is to simply allow str for PathLike, which gives this result: os.PathLike[str]

PathLike with the implemented fix
Actual code without any errors
(awesome! This issue's no longer a thing with this fix!)

There has some discussion about the same situation on this StackOverflow page: https://stackoverflow.com/a/69680089/13720113

I can make a PR for fixing this (Of course, with some testing, since there is Python version checks), as long as this issue is approved.

Thank you, hoping this issue get sorted soon!

@Byron Byron added acknowledged help wanted c-typing Mypy causes warnings or errors when run labels Jul 27, 2022
@Byron
Copy link
Member

Byron commented Jul 27, 2022

Thanks for bringing this up. I think for correctness, the type should be os.PathLike[str|bytes] if this doesn't pose a problem. A PR for that would definitely be welcome, thanks for offering.

@madebylydia
Copy link
Contributor Author

madebylydia commented Jul 28, 2022 via email

@madebylydia
Copy link
Contributor Author

So, after digging thing a bit deeper, my error I had in #1474 was because I was trying to convert git.repo.Repo.working_tree_dir (With the "corrected" type hint) to a pathlib.Path object (Which should implement os.PathLike too), but pathlib.Path refuses the corrected type hint with bytes because pathlib.Path does not support bytes when creating a new instance of it.

image

My wonder here is why, actually, should I add bytes?
I sure would have not questioned if it was all good, but as for right now, I have wonder since pathlib.Path does not accept it...

@Byron
Copy link
Member

Byron commented Aug 7, 2022

Thanks for the follow-up. bytes are the correct choice on unix platforms which represent paths as bytes, no encoding can be assumed for that reason which would be the case for paths that are str, which in python is an interpreter controlled encoding that can change depending on many factors. It appears that Python made its choice and given the history of encoding errors I have witnessed with GitPython in the past decade I wonder what they are thinking. Now there seems to be no way around it being broken.

Accepting that, then not adding bytes would be the way to go and leaving the PR as is, right? If so, please feel free to remove bytes once again and it should be OK to merge. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledged c-typing Mypy causes warnings or errors when run help wanted
Development

Successfully merging a pull request may close this issue.

2 participants