-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
mark: expose Mark, MarkDecorator, MarkGenerator under pytest for typing purposes #8179
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
Conversation
The type cannot be constructed directly, but is exported for use in type annotations, since it is reachable through existing public API.
The type cannot be constructed directly, but is exported for use in type annotations, since it is reachable through existing public API.
The type cannot be constructed directly, but is exported for use in type annotations, since it is reachable through existing public API.
_param_ids_generated = attr.ib( | ||
type=Optional[Sequence[str]], default=None, repr=False | ||
) | ||
_param_ids_generated: Optional[Sequence[str]] = attr.ib(default=None, repr=False) |
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.
should we introduce a metaclass that mangles the _ispytest
in __call__
- making all those classes have strange ctors is a pain
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.
IMHO I prefer the _is_pytest
approach because it is more explicit... I think a metaclass introduces needless complexity just for this change.
Just stating my opinion though, not against changing it to a metaclass if Ran also prefers to do so.
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 feel simillary to @nicoddemus, I think that while the _ispytest
is a bit annoying, it is better in most other dimensions (like ease of understanding, greppability, ease of type checking, ease of bypassing when needed), so I prefer it.
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.
Great work!
@@ -465,9 +488,6 @@ def test_function(): | |||
applies a 'slowtest' :class:`Mark` on ``test_function``. | |||
""" | |||
|
|||
_config: Optional[Config] = None | |||
_markers: Set[str] = set() |
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.
Ouch!
This is part of #7469, please see details there. Similar to previous PR #8017 that went in 6.1; haven't seen any complaints about it yet.