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

context_management: add a type stub override to fix typing #457

Merged

Conversation

gotmax23
Copy link
Contributor

Type checkers (mypy, pyright, et al.) don't understand the ContextManager descriptor class.
With a stub file, we can tell the type checker to treat ContextManager as a simple decorator function which is something that it understands.

from typing import reveal_type

import specfile

s = specfile.Specfile("./fedora/python-specfile.spec")

# Before: types.MethodType
# After: (allow_duplicates: bool = False, default_to_implicit_numbering: bool = False, default_source_number_digits: int = 1) -> GeneratorContextManager[Sources]
reveal_type(s.sources)
# Before: Any
# After: GeneratorContextManager[Sources]
reveal_type(s.sources())
# Before: Any
# After: Sources
reveal_type(s.sources().__enter__())
reveal_type(s.sources().content)

RELEASE NOTES BEGIN

context_management: add a type stub override to fix typing. Type checkers like mypy and pyright can now correctly determine the types for .sources(), .sections(), and the other Specfile methods that return context managers.

RELEASE NOTES END

Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/4cd4cae9138b43e1a0d942978b61b75d

✔️ pre-commit SUCCESS in 1m 54s
✔️ specfile-tests-rpm-deps SUCCESS in 1m 05s
✔️ specfile-tests-pip-deps SUCCESS in 1m 02s

Copy link
Member

@nforro nforro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks!

Could you just add the copyright header to the new file?

@nforro
Copy link
Member

nforro commented Mar 11, 2025

/packit build

@nforro
Copy link
Member

nforro commented Mar 11, 2025

/packit-stg build

@nforro
Copy link
Member

nforro commented Mar 11, 2025

/packit rebuild-failed

@nforro
Copy link
Member

nforro commented Mar 11, 2025

/packit-stg rebuild-failed

Type checkers (mypy, pyright, et al.) don't understand the
ContextManager descriptor class.
With a stub file, we can tell the type checker to treat ContextManager
as a simple decorator function which is something that it understands.

``` python
from typing import reveal_type

import specfile

s = specfile.Specfile("./fedora/python-specfile.spec")

\# Before: types.MethodType
\# After: (allow_duplicates: bool = False, default_to_implicit_numbering: bool = False, default_source_number_digits: int = 1) -> GeneratorContextManager[Sources]
reveal_type(s.sources)
\# Before: Any
\# After: GeneratorContextManager[Sources]
reveal_type(s.sources())
\# Before: Any
\# After: Sources
reveal_type(s.sources().__enter__())
reveal_type(s.sources().content)
```
@gotmax23 gotmax23 force-pushed the contextmanager-type-check branch from a604ec9 to 51565f1 Compare March 11, 2025 19:51
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/8a82590f38fc45cebb92378cb2640e4f

✔️ pre-commit SUCCESS in 1m 52s
✔️ specfile-tests-rpm-deps SUCCESS in 1m 01s
✔️ specfile-tests-pip-deps SUCCESS in 59s

@nforro
Copy link
Member

nforro commented Mar 12, 2025

/packit build

@nforro
Copy link
Member

nforro commented Mar 12, 2025

/packit-stg build

@nforro nforro added the mergeit Zuul, merge it! label Mar 12, 2025
@nforro
Copy link
Member

nforro commented Mar 12, 2025

regate

Copy link
Contributor

Build succeeded (gate pipeline).
https://softwarefactory-project.io/zuul/t/packit-service/buildset/93cba1360ed944b8948f14541dfed06b

✔️ pre-commit SUCCESS in 1m 59s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit eab3524 into packit:main Mar 12, 2025
38 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit Zuul, merge it!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants