Fix mypy#140
Merged
Merged
Conversation
Python 3 does not require inheriting from object anymore.
Incomplete signatures were behind four of the remaining mypy errors.
The first overload returned T without taking it as a parameter, which mypy rejected and made the second overload unreachable.
The `fn` overload can only be passed positionally by the real implementation, and the wrapper can return an awaitable, so the declared types now match runtime behavior instead of fighting mypy.
mypy flags __args__ as private since type doesn't declare it, so use the public get_args API instead.
Strict mode implies these flags so they can be removed, and it exposed one real typing issue that needed fixing.
Owner
|
Great fixes, thanks. Do you plan to change anything else? Otherwise, this can be a solid next version. |
Contributor
Author
No other changes planned at the moment 😅 Thank you for merging these PRs 😃 |
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
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.
Hi again,
This is a follow-up to #139. It makes the red
Type checking (library code)job green. On top of that it enables mypy strict mode. Like in #136, I worked on this with AI assistance (Claude).Python 3.10 baseline cleanup
After dropping Python 3.9 (#137), some compatibility code is not needed anymore: the PEP 604/560 version checks, the
typing_extensionsParamSpec fallback, and the privatetyping._GenericAlias/typing._Unionimports (replaced with the publict.get_origin()/t.get_args()).Type hints now use the PEP 604 union syntax (
X | None). I also removed the matching pyupgrade ignores from the ruff config, so ruff keeps checking this.Small find on the way: an earlier formatting commit changed the test annotation
C | Nonetot.Optional[C]. So the PEP 604 test did not test PEP 604 anymore. I restored it. Now both spellings have their own test.Mypy: 16 errors to zero, then strict
Fixed in small commits so each one is easy to review:
t.Callable, the untypedattr()implementation)Injector.get_instancenow has the sametype[T]overload as the module-levelinstance(), with a newtyping_checksguard like we added in fix: restore static typing ofinject.attr(Cls)#136autoparamsfn-overload is positional-only now. mypy was right here: the old type hints allowedautoparams(fn=...), but that call crashes at runtime. This only changes the type hints: at runtime,autoparams(fn=...)always raised a TypeError, before and aftert.get_args()instead of.__args__After that, mypy was at zero errors. So I enabled
strict = true(this resolves the old TODO) and removed the single flags that strict already includes. Strict found one more wrong annotation in a private helper, which is also fixed.There are no runtime behavior changes. The test matrix stays green on 3.10-3.14.
README
One small fix: the "Keys" sample was a SyntaxError (a missing dot between the chained
bindcalls). I also ran all the other README samples. Some more are outdated (the context-managers sample and the classmethod +inject.attrexamples). I might fix those in a separate PR as this one is big already.Suggestion: Make CI jobs mandatory before merging
In the recent PRs we locked the dependencies and cleaned up the code base and the CI. So new tool releases cannot break the CI anymore, a red job now means a real problem. And with this PR all jobs are green. I think this is a good moment to make the CI jobs mandatory for merging.
Thanks and all the best 😄
Elias