-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
feat: expose FixtureFunctionDefinition for typing #14862
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Exposed :class:`pytest.FixtureFunctionDefinition` as the type of a fixture function after decoration by :func:`pytest.fixture`, allowing fixture factories to use it in type annotations. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1454,6 +1454,12 @@ def __call__(self, function: FixtureFunction) -> FixtureFunctionDefinition: | |
|
|
||
| # TODO: paramspec/return type annotation tracking and storing | ||
| class FixtureFunctionDefinition: | ||
| """The type of a fixture function after decoration by :func:`pytest.fixture`. | ||
|
|
||
| This type is public for type annotations. It should not be instantiated | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This paragraph isn't needed, we have runtime protection against this, so better to keep things short. |
||
| or subclassed by users, and its attributes are not part of the public API. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should probably be marked
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. I had considered whether |
||
| """ | ||
|
|
||
| def __init__( | ||
| self, | ||
| *, | ||
|
|
||
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.
@pytest-dev/core if we expose this - do we want to make the return type a generic?
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.
This seems directly related to the TODO introduced in #12473. I found #13036, which explored preserving both the
ParamSpecand return type, and it was closed due to lack of time rather than because the design was rejected.Since the generic arity and semantics would become part of the public API, I agree that this is worth settling before exposing the type. Do we want only a declared return-type parameter, or the
ParamSpecplus return-type shape from #13036? For yield fixtures, should the type parameter represent the declaredGenerator[...]return type or the yielded fixture value?I’ll wait for input from @pytest-dev/core before making further changes.
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.
I think it makes sense to make the return type generic if possible.
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.
this seems quite a lot like a gpt driven rely - i think we may need to alter ai contribution policy a bit more as it increasingly difficult to distinguish good unattended model and attended model eagerly doing things on behalf of the user
Uh oh!
There was an error while loading. Please reload this page.
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.
umm... I did use ai to help write the reply, but I reviewed it before posting. However, reading it again now, I can see that it sounds too ai assisted. sorry about that.
I found 13036 while looking into whether the return type should be generic, because it explored carrying the fixture function's parameter and return types through
FixtureFunctionDefinition. That's where theParamSpecquestion came from.