Fix mypy errors in testcommon py #29179
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -324,11 +324,13 @@ def test_is_datetimelike(): | |
|
|
||
| @pytest.mark.parametrize( | ||
| "dtype", | ||
| [ | ||
| type(pd.Series([1, 2])), | ||
| *ALL_INT_DTYPES, | ||
| *to_numpy_dtypes(ALL_INT_DTYPES), | ||
| *ALL_EA_INT_DTYPES, | ||
| *to_ea_dtypes(ALL_EA_INT_DTYPES), | ||
| ], | ||
| ) | ||
| def test_is_integer_dtype(dtype): | ||
| assert com.is_integer_dtype(dtype) | ||
|
|
@@ -352,11 +354,13 @@ def test_is_not_integer_dtype(dtype): | |
|
|
||
| @pytest.mark.parametrize( | ||
| "dtype", | ||
|
There was a problem hiding this comment. what is mypy's complaint about this? I think the current idiom is more beginner-friendly than the args-expansion idiom There was a problem hiding this comment. IIRC the issue is that you can't really do list addition and have it type inference appropriately all in one pass, at least not from a static analysis perspective Instead of doing this could you also just initialize this a line up with the appropriate type comment and just pass that variable into the parametrization? There was a problem hiding this comment. @WillAyd Just so I understand exactly what you're suggesting, are you talking about initializing the list of dtypes originially constructed by list addition through unpacking them into a list outside of the parametrizations, giving that list a type stub (presumably List[Dtype]), and then passing it into the paramaterization? There was a problem hiding this comment. No unpacking just defined this as a variable on the line above. signed_int_dtypes = [] # type: List[Union[pd.Series, Dtype]]And then just use that as the argument here. Side note - I'm assuming the type above works but can't remember what the mypy complaint was and what it should be just glancing at it. Might need to tweak depending on what you see |
||
| [ | ||
| type(pd.Series([1, 2])), | ||
| *SIGNED_INT_DTYPES, | ||
| *to_numpy_dtypes(SIGNED_INT_DTYPES), | ||
| *SIGNED_EA_INT_DTYPES, | ||
| *to_ea_dtypes(SIGNED_EA_INT_DTYPES), | ||
| ], | ||
| ) | ||
| def test_is_signed_integer_dtype(dtype): | ||
| assert com.is_integer_dtype(dtype) | ||
|
|
@@ -384,11 +388,13 @@ def test_is_not_signed_integer_dtype(dtype): | |
|
|
||
| @pytest.mark.parametrize( | ||
| "dtype", | ||
| [ | ||
| type(pd.Series([1, 2], dtype=np.uint32)), | ||
| *UNSIGNED_INT_DTYPES, | ||
| *to_numpy_dtypes(UNSIGNED_INT_DTYPES), | ||
| *UNSIGNED_EA_INT_DTYPES, | ||
| *to_ea_dtypes(UNSIGNED_EA_INT_DTYPES), | ||
| ], | ||
| ) | ||
| def test_is_unsigned_integer_dtype(dtype): | ||
| assert com.is_unsigned_integer_dtype(dtype) | ||
|
|
||
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.
Is this change to
typeintentional?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.
Yes. If I understand the list correctly then a Series is a data type that should be accounted for in these tests, right?
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 can see where the naming is confusing but you should keep that the same -
is_integer_dtypeworks on types and arraysThere 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.
Alright, I reverted that change.