-
Notifications
You must be signed in to change notification settings - Fork 168
chore(typing): Add _typing_compat.assert_never
#2717
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dangotbanned - should we replace all the other "unreachable" code paths as well (search)?
One nice thing that we are losing is the suggestion "Please report an issue at https://github.com/narwhals-dev/narwhals/issues/new". I wonder if it's worth it to have our version of assert_never independently of the python version
Thanks for peeking @FBruzzesi
If you can find any (besides db14ab1) that both We can only use it when we can prove that every case has been checked, like this It isn't just limited to Here for example, the type hasn't been narrowed to
I'm open to using a custom message - only caveat is it has to be the same for every use. We need to have the signature as this to be able to link whatever we use back to def assert_never(arg: Never, /) -> Never:I was playing around with the idea of different runtime behavior than the from __future__ import annotations
import sys
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from typing import assert_never
if sys.version_info >= (3, 11):
from typing import Never
else:
from typing_extensions import Never
else:
def assert_never(arg: Never, /) -> Never:
msg = "Hello"
raise AssertionError(msg) |
I will give it a shot π€
I would suggest to simply append the link at the end of the message: def assert_never(arg: Never, /) -> Never:
value = repr(arg)
if len(value) > _ASSERT_NEVER_REPR_MAX_LENGTH:
value = value[:_ASSERT_NEVER_REPR_MAX_LENGTH] + "..."
msg = (
f"Expected code to be unreachable, but got: {value}.\n"
"Please report an issue at [narwhals-dev/narwhals/issues/new](https://github.com/narwhals-dev/narwhals/issues/new)"
)
raise AssertionError(msg) |
| _BUG_URL = ( | ||
| "https://github.com/narwhals-dev/narwhals/issues/new?template=bug_report.yml" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FBruzzesi I tried using the link you gave first, but I think this one might be more handy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes nice one! Thank you π
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, thanks @dangotbanned !




What type of PR is this? (check all applicable)
Related issues
is_betweendoesn't showClosedIntervalvalues. Remove type aliasesΒ #2715 (comment)AssertionErrorinternallyΒ #1894Checklist
If you have comments or can explain your changes, please do so below