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

Wrong type annotation on Index.commit for parent_commits #1866

Closed
mshonichev opened this issue Mar 12, 2024 · 3 comments · Fixed by #1859
Closed

Wrong type annotation on Index.commit for parent_commits #1866

mshonichev opened this issue Mar 12, 2024 · 3 comments · Fixed by #1859

Comments

@mshonichev
Copy link

With GitPython==3.1.42 one can observe following annotation of parent_commits argument to IndexFile.commit ([1]):

...
parent_commits: Union[Commit_ish, None] = None,
...

which is obviously not correct, because that argument just passed unmodified downwards to Commit.create_from_tree, which has following annotation instead ([2]):

parent_commits: Union[None, List["Commit"]] = None

That leads to nasty Mypy warnings when one's trying to use IndexFile.commit with anything different than None.


[1] - https://github.com/gitpython-developers/GitPython/blob/3.1.42/git/index/base.py#L1080
[2] - https://github.com/gitpython-developers/GitPython/blob/3.1.42/git/objects/commit.py#L506

@Byron
Copy link
Member

Byron commented Mar 12, 2024

Thanks for reporting!

I think this issue might be fixed once #1859, which touches on what constitutes Commit_ish. @EliahKagan certainly has a much better understanding of the matter though, and I wouldn't want the PR to become larger than it has to be just to include an actual fix (if it's not fixed naturally).

@EliahKagan
Copy link
Contributor

Yes, this is part of what #1859 fixes (b4b6e1e).

@mshonichev
Copy link
Author

Great! Wow, just understood, that @Byron at stack overflow threads related to GitPython is actually the package maintainer, so silly of me :). Nice to meet, looking forward PR

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

Successfully merging a pull request may close this issue.

3 participants