Improve ambiguous type annotations #22
Conversation
- Ditched the type aliases that were causing the TypeVar parsing to fail. - Made the decorated AttrWrapper generic
We now rely on the ParamSpec and Concatenate features of PEP 612 which weren't implemented until Python 3.10. In order to use those features while supporting Python 3.9, we can temporarily replace any typing imports with backported features from typing-extensions. Once we drop 3.9 support, we can switch back to typing and drop the dependency.
There was a problem hiding this comment.
@aazuspan, very cool stuff. I was able to recreate your initial writeup after I activated python.analysis.inlayHints.variableTypes and it worked perfectly.
I think I'd lean toward pushing this now and relying on typing_extensions, but admittedly I don't know the negative consequences of doing that. Like you, it seems to make sense to me to get at least a single release at 3.9 before pulling support for it. That hasn't seemed to impact us too much recently.
Thanks for improving the development experience!
To infer the correct type when `as_dataset` is unspecified, we shouldn't give it a default value in the `True` case.
The number of output targets for kneighbors must be n_neighbors when it's specified.
|
Thanks for the review and catching my mistakes @grovduck! I just pushed a few commits that should fix the outstanding issues with the
Makes sense to me, thanks for weighing in! I think I'm overly cautious about adding dependencies, and there shouldn't be any practical issues with using |
Sorry to be dense here, but now that you have The change to (and new test for) |
Only to support extended |
|
Oh yeah, that one! All looks good. |
I noticed some ambiguous types being inferred in VS Code during usage, so I spent a little time trying to improve those. That basically boiled down to:
@overloadto specify return types that depend on input parameters andTo demonstrate, here are the inferred types in grey (activated with the
python.analysis.inlayHints.variableTypessetting in VS Code) before the typing changes:And after:
Aside from adding some code complexity, the downside of these changes that I realized too late is that PEP 612 wasn't implemented until Python 3.10, so I had to temporarily add
typing-extensions(an official backport of new typing features) as a dependency so that we can continue supporting 3.9. Once we eventually drop 3.9, we could also droptyping-extensions.The other alternatives would be to hold off on this PR or drop 3.9 support now. The Numpy support schedule recommended dropping 3.9 support a couple months ago, so that's probably not unreasonable, but it does feel a little restrictive considering we don't have any older releases for compatibility. Let me know if you have any strong feelings either way @grovduck!