-
Notifications
You must be signed in to change notification settings - Fork 139
Added initial typehints #494
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
Open
danielmorell
wants to merge
18
commits into
master
Choose a base branch
from
added/typehints
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
3603881
Removed support for Python 3.6/3.7 missing ContextVar
danielmorell 2414d2a
Initial work on adding typing.
danielmorell 4813578
Merge branch 'master' into added/typehints
danielmorell 57d5945
Added type stubs and typed imports where possible ignored the rest
danielmorell eb58125
Merge branch 'master' into added/typehints
danielmorell 93194dc
Merge branch 'master' into added/typehints
danielmorell 9ef5009
Added initial round of typehints
danielmorell 158f79e
Fixed missing route passed to app init.
danielmorell 2312565
Fixed check_level function to allow custom levels.
danielmorell a8102b7
Removed commented code.
danielmorell 83434fc
Address review comments.
danielmorell a02b8c0
Fixed deprecated Starlette routing in tests.
danielmorell 1a3c08d
More code review fixes.
danielmorell e15edf1
Removed unnecessary missing contextvar test.
danielmorell 6318c76
Addressed code review comments.
danielmorell 3fe9121
Dropped support for Python 3.9
danielmorell f347608
Updated 3.9 Unions and Optional to modern syntax
danielmorell 50e8438
Moved type only imports to behind typing.TYPE_CHECKING.
danielmorell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 The classifiers list in pyproject.toml still includes
"Programming Language :: Python :: 3.9"even though this PR bumpedrequires-pythonto>=3.10(and the CI matrix + README were updated to drop 3.9). This is metadata-only (pip usesrequires-pythonas authoritative), but it makes PyPI's displayed 'Compatible Python versions' misleading. Fix: delete theProgramming Language :: Python :: 3.9line from the classifiers array around line 15.Extended reasoning...
What the bug is
pyproject.tomlline 15 still declares:"Programming Language :: Python :: 3.9",while line 42 was changed in this PR to
requires-python = ">=3.10". The CI matrix in.github/workflows/ci.ymlwas likewise narrowed to 3.10–3.14, andREADME.mdwas updated to advertise 1.4.0 as supporting 3.10–3.14 only. So the classifier is the one piece of metadata that didn't get propagated when 3.9 support was dropped.How it manifests
On PyPI, the project page's Meta → Classifiers section will list
Python :: 3.9alongside 3.10–3.14, suggesting that 3.9 is supported. A user on Python 3.9 reading that classifier and runningpip install rollbarwill then be refused by pip withERROR: Package 'rollbar' requires a different Python: 3.9.x not in '>=3.10', becauserequires-python(the authoritative field pip consults) excludes 3.9. So the symptom is a metadata/install mismatch, not a runtime failure.Why existing code doesn't prevent it
Nothing in the build pipeline cross-checks classifiers against
requires-python. They are independent free-form strings in PEP 621 metadata; the build backend won't flag the inconsistency, and mypy (the newtypesjob introduced in this PR) doesn't touchpyproject.tomlsemantics either. The commit that dropped 3.9 (3fe9121) bumpedrequires-python, removed 3.9 from the CI matrix, and updated the README, but did not delete the classifier line — leaving this single line stale.Impact
Documentation/metadata only.
pipcorrectly refuses 3.9 installs becauserequires-pythontakes precedence, so no runtime regression. The cost is purely user-facing: a misleading PyPI display and a small inconsistency between PR-internal sources of truth.How to fix
Remove the single line from the classifiers array in
pyproject.toml(around line 15):"Programming Language :: Python", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11",No other changes needed; the surrounding classifiers,
requires-python, README, and CI matrix are already consistent at 3.10–3.14.Step-by-step proof
pyproject.tomllines 12–22: classifiers includesProgramming Language :: Python :: 3.9andProgramming Language :: Python :: 3.10through3.14.pyproject.tomlline 42 in this PR:requires-python = ">=3.10".README.mdline 27 in this PR: '1.4.0 | 3.10, 3.11, 3.12, 3.13, 3.14 | Full'..github/workflows/ci.ymlline 15 in this PR:python-version: ['3.10', 3.11, 3.12, 3.13, 3.14](3.9 removed).