-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Type annotate Metafunc #6735
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
Type annotate Metafunc #6735
Conversation
@@ -1000,49 +1023,59 @@ def _resolve_arg_ids( | |||
:rtype: List[str] | |||
:return: the list of ids for each argname given | |||
""" | |||
idfn = None |
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 rearranged the code here to make it amenable to mypy. I also think it's a bit clearer.
try: | ||
len(ids) |
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 had to refactor a bit to make it amenable to mypy. It's meant to be equivalent.
except TypeError: | ||
raise TypeError("ids must be a callable, sequence or generator") | ||
else: | ||
import itertools |
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 moved this import up -- don't see why it should be lazy.
@@ -1095,7 +1132,7 @@ def _validate_if_using_arg_names(self, argnames, indirect): | |||
pytrace=False, | |||
) | |||
else: | |||
if isinstance(indirect, (tuple, list)): | |||
if isinstance(indirect, Sequence): |
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.
Generalized this a bit to make typing easier. It's also what other functions already check - was inconsistent.
@@ -1113,17 +1154,15 @@ def _validate_explicit_parameters(self, argnames, indirect): | |||
:param indirect: same ``indirect`` parameter of ``parametrize()``. | |||
:raise ValueError: if validation fails | |||
""" | |||
if isinstance(indirect, bool) and indirect is True: |
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.
Had to rearrange things here a bit.
@@ -1204,13 +1253,22 @@ def _idval(val, argname, idx, idfn, item, config): | |||
return ascii_escaped(val.pattern) | |||
elif isinstance(val, enum.Enum): | |||
return str(val) | |||
elif hasattr(val, "__name__") and isinstance(val.__name__, str): | |||
elif isinstance(getattr(val, "__name__", None), str): |
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.
Changed a bit to avoid mypy attr warnings.
for marker in metafunc.definition.iter_markers(name="parametrize"): | ||
metafunc.parametrize(*marker.args, **marker.kwargs, _param_mark=marker) | ||
# TODO: Fix this type-ignore (overlapping kwargs). |
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.
Do you have a plan for this?
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.
Not really. There's a workaround of creating kwargs = {**marker.kwargs, '_param_marker': marker}
but I think it's worse than the disease.
99ce6ac
to
5945c3f
Compare
Thanks @blueyed, I applied the changes. |
Extracted from #6717.