Add py.typed marker so type checkers see spiceypy's annotations#552
Open
Sammy-Dabbas wants to merge 1 commit into
Open
Add py.typed marker so type checkers see spiceypy's annotations#552Sammy-Dabbas wants to merge 1 commit into
Sammy-Dabbas wants to merge 1 commit into
Conversation
Adds the PEP 561 marker, the Typing :: Typed classifier, and an sdist.include entry (the existing globs only match *.py/*.pyx). The wheel picks the marker up automatically via scikit-build-core package auto-detection. Fixes AndrewAnnex#415.
Owner
|
@Sammy-Dabbas my apologies but I only just noticed the pull request, thanks for thinking to contribute! I will review this more in depth over the coming week. My only initial thought though is that point on rechecking the current annotations first before including this change because otherwise users will experience not a lot of benefit if mypy is indicating errors due to incorrect type annotations. This in the past was going to be a very tedious task but is pretty tractable with improved linter tools and ai tools available. But once I have some time to sit and think on it I will see if I want to stick to that requirement or not. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #415.
This adds the PEP 561 py.typed marker plus the two packaging lines needed to ship it: the Typing :: Typed classifier and an sdist.include entry, since the existing include globs only match *.py and *.pyx. The wheel side needs no change because scikit-build-core's package auto-detection picks up non-Python files inside src/spiceypy; I confirmed that with a minimal repro pinned to the same scikit-build-core version, and verified the built sdist contains src/spiceypy/py.typed.
On the concern from the 2021 discussion about internal mypy errors: shipping the marker does not expose them to users. mypy does not re-check site-packages internals when following imports, so downstream projects only see the public annotations. I demonstrated the user-facing effect in a fresh venv against the 8.1.2 wheel: before the marker, mypy reports import-untyped and silently ignores two deliberate type errors in user code calling spiceypy; with the marker present, both are caught ([assignment] and [arg-type]). A mypy pass over spiceypy's own internals would be worthwhile as an independent follow-up, but it is not a prerequisite for the marker.