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

Add type annotations for @intrinsic #9401

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

aneeshdurg
Copy link

Previously using the intrinsic decorator would cause typing issues when invoking the decorated function, e.g.:

@intrinsic()
def myIntrinsic(typing_context, a, b):
   ...
   
myIntrinsic(a, b) # type checker complains that there's a missing parameter

This can be solved by annotating intrinsic with types that indicate that the decorated function does not take the typing context parameter.

@aneeshdurg aneeshdurg force-pushed the type_annotations_for_intrinsic branch from 80bd6b9 to 8465419 Compare January 23, 2024 18:14
@aneeshdurg
Copy link
Author

the failure in the docs build seems to be because python version before 3.10 did not have typing.Concatenate. This can by resolved by either taking typing-extensions as a dependency, or possibly moving the annotations into a stub file and making it conditional on the python version. Will try to test that out soon.

@aneeshdurg
Copy link
Author

I was able to resolve the CI issues by moving the annotations to a .pyi file. This allows importers of numba.core.extending to gain type information about intrinsic, but doesn't actually do any type enforcement within extending.py.

@aneeshdurg aneeshdurg force-pushed the type_annotations_for_intrinsic branch from 4a281c3 to 98d9939 Compare January 30, 2024 03:24
@aneeshdurg
Copy link
Author

@stuartarchibald now that this is passing tests can the review process proceed?

@stuartarchibald
Copy link
Contributor

@stuartarchibald now that this is passing tests can the review process proceed?

@aneeshdurg yes, I've now queued it for review. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants