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 py.typed for mypy compatibility #415

Open
GeorgeEbberson opened this issue Jul 2, 2021 · 2 comments
Open

Add py.typed for mypy compatibility #415

GeorgeEbberson opened this issue Jul 2, 2021 · 2 comments
Labels
enhancement not a bug this issue is not a bug

Comments

@GeorgeEbberson
Copy link

GeorgeEbberson commented Jul 2, 2021

Is your feature request related to a problem? Please describe.
Although the entire library is typed, when importing it into other projects as a third-party dependency mypy cannot analyse it because it needs the py.typed file adding (see PEP-561).

Describe the solution you'd like
Add an empty py.typed file in the top-level of the directory, as well as adding it to the package_data in setup.py as indicated in the PEP.

Describe alternatives you've considered
Alternative option would be to go through and add .pyi type stubs for each of the .py files, which would be entirely redundant when all the information is already in the .py files already.

EDIT:
In doing this, it would be lovely to use the @overload decorator (docs) to encode the different call signatures of functions. Although, this would be a bit more work, it would remove a lot of the errors that things like mypy raise when you're using e.g. the non-vectorised version of a function and the return type is a union of the vectorised and non-vectorised forms.

@AndrewAnnex
Copy link
Owner

Hey @GEbb4, yeah I agree these are both good ideas. I hadn't realized that the py.typed filed was required before, and I was worried when I first saw this issue that it would require making stub files (as I greatly prefer the inline type annotations), so that looks easy to fix. I think I can add mypy checking to the github tests fairly quickly as another enabling component to this, I assume there are many errors will be hard/annoying to fix but I can take my time at fixing them.

@AndrewAnnex
Copy link
Owner

AndrewAnnex commented Jul 3, 2021

well, talk about a quick turn around but I think there maybe some fundamental issues with how spiceypy is implemented that will make it very difficult to pass mypy checks. Firstly, there appears to be some missing bits around ctypes and mypy that can't be fixed but more critically the way I have written each wrapper function to override the function parameters when casting them to ctypes fails the "Incompatible types in assignment" check. Mypy treats parameters as static types. Fixing this would require refactoring each wrapper function to not override the parameters by creating new variables with new names. This would be really time consuming without an automated approach and I think it would be non trivial to implement.

I still think that this is possible to do, but I think this will need to remain "on the back-burner" as a non critical feature addition. Additional help to resolve these issues would speed this along.

@AndrewAnnex AndrewAnnex added enhancement not a bug this issue is not a bug labels Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement not a bug this issue is not a bug
Projects
None yet
Development

No branches or pull requests

2 participants