-
Notifications
You must be signed in to change notification settings - Fork 793
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
add typehint Any to Undefined object #2670
Conversation
Can someone please have a look at this? Right now checking code with pylance (vs code default) or pyright (same thing really) produces an error for every argument in every call in altair. I'm not exactly sure if this (auto-assumption that argument has a type of its default value) is the PEP-compatible behavior, but this simple trick proposed by @brahn probably can solve the issue. |
Thanks for the ping @arogozhnikov! Do you know anyone who can perform a review regarding the dos and do nots for types to push this forward? |
I agree this reads a bit odd a bit odd at first! To provide a bit of background:
To explain why this change helps, it's useful to look at an example like the following https://github.com/altair-viz/altair/blob/fc7f5510d189303aa58e751cb5876fb4929f36c8/altair/vegalite/v5/schema/mixins.py#L11-L15 The arguments to this function all have the By adding the type hint that |
@brahn made a nice summary. My two cents: From user perspective, I don't see a case when this can turn into a problem. Looking forward, at some point I expect
I'm quite certain that second option is preferred. |
As usual, things are not that easy. The essence of this issue has been, albeit not directly named, discussed in https://twitter.com/jakevdp/status/1557778437493755904 & https://twitter.com/jakevdp/status/1518659999844630528. |
@mattijn am I following correctly that you'd like to see something like this? Happy to update the PR with this change |
closing in favor of #2681 |
This PR adds the typehint
Any
to the undefined object, suppressing the many type errors described in microsoft/pylance-release#3210Candidly this is not a great solution and hardly sufficient to address #2493 (I think a full set of type hints like #2614 would be preferable) but it at least this removes the distraction.
Very open to alternatives. Thanks for your consideration!
cc: @thewchan