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

assert_never raises different exception than documented #1857

Closed
EliahKagan opened this issue Mar 4, 2024 · 1 comment · Fixed by #1859
Closed

assert_never raises different exception than documented #1857

EliahKagan opened this issue Mar 4, 2024 · 1 comment · Fixed by #1859

Comments

@EliahKagan
Copy link
Contributor

git.types.assert_never is documented to raise AssertionError if raise_error is True and exc is None:

If raise_error is True, will also raise AssertionError or the Exception passed to exc.

But it actually raises ValueError in this situation, its full implementation being:

GitPython/git/types.py

Lines 85 to 89 in b2c3d8b

if raise_error:
if exc is None:
raise ValueError(f"An unhandled Literal ({inp}) in an if/else chain was found")
else:
raise exc

For comparison. there is an assert_never function in Python's typing module since 3.11 (and in typing_extensions). This function is similar but only has one parameter--which the first parameter of git.types.assert_never corresponds to--and it documents that it raises an exception if called at runtime, but does not document the type of exception raised. At least currently in Python 3.12, it raises AssertionError.

The GitPython function is a public member of git.types, and it is used internally in GitPython. I don't know if the docstring should be fixed to describe the existing behavior, or if that behavior should itself be considered the bug here and changed to match the docstring.

@Byron
Copy link
Member

Byron commented Mar 5, 2024

Thanks for letting me know.

It does seem a bit strange to me that functions related to testing are part of the git public API in the first place, but it's probably nothing that can truly be fixed anymore.

With that said, even tough there mere existence could be considered a bug, I tend to leave it and its behaviour alone while fixing the docstring.

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.

2 participants