Fix mypy errors in testcommon py #29179

HawkinsBA · 2019-10-23T05:12:29Z

Apply black formatting to test_common.py

WillAyd · 2019-10-23T15:20:01Z

Can you post the error this solves? Does annotating these instead of unpacking change anything?

HawkinsBA · 2019-10-23T21:39:19Z

pandas\tests\dtypes\test_common.py:327: error: Unsupported operand types for + ("List[Series]" and "List[str]")
pandas\tests\dtypes\test_common.py:355: error: Unsupported operand types for + ("List[Series]" and "List[object]")
pandas\tests\dtypes\test_common.py:387: error: Unsupported operand types for + ("List[Series]" and "List[str]")

In the original file it looks like both lists of Strings and Dtypes (ALL_INT_DTYPES,and to_numpy_dtypes(ALL_INT_DTYPES)) are are concatenated to a List[Series], so my rationale behind unpacking these is to preserve the resulting list of both of these list types being concatenated without the "+" operator.

Would it be better to approach this problem preserving concatenation with type annotations?

WillAyd

OK might just have to live with the unpacking then

WillAyd · 2019-10-24T00:03:05Z

pandas/tests/dtypes/test_common.py

-    + ALL_EA_INT_DTYPES
-    + to_ea_dtypes(ALL_EA_INT_DTYPES),
+    [
+        type(pd.Series([1, 2])),


Is this change to type intentional?

Yes. If I understand the list correctly then a Series is a data type that should be accounted for in these tests, right?

I can see where the naming is confusing but you should keep that the same - is_integer_dtype works on types and arrays

Alright, I reverted that change.

Removed type(pd.Series[...])

jbrockmendel · 2019-10-28T19:17:49Z

@HawkinsBA can you rebase

* xfail on numpy 1.18 * CI: try using numpy wheel

* API: Restore getting name from MultiIndex level xref https://issues.apache.org/jira/browse/ARROW-6922 / pandas-dev#27242 (comment) / pandas-dev#29032 No docs yet, since it isn't clear how this will eventually sort out. But we at least want to preserve this behavior for 1.0 * fixups

…v#28993)

Closes pandas-dev#21624

)

…pandas-dev#29096)

pandas-dev#29084)

…-dev#29083)

pep8speaks · 2019-11-03T04:39:01Z

Hello @HawkinsBA! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-11-07 01:14:18 UTC

HawkinsBA · 2019-11-03T05:19:40Z

So without Any in the type annotation I get the following MyPy errors while the Azure Pipelines run:

pandas/tests/dtypes/test_common.py:336: error: Unsupported operand types for + ("List[Union[Series, str, str, Any, ExtensionDtype, ExtensionDtype]]" and "List[str]") pandas/tests/dtypes/test_common.py:404: error: Unsupported operand types for + ("List[Union[Series, str, str, Any, ExtensionDtype, ExtensionDtype]]" and "List[str]")

However when Any is included in the type annotation I get the following errors:

pandas/tests/dtypes/test_common.py:336: error: Unsupported operand types for + ("List[Union[Series, str, str, Any, ExtensionDtype, Any, ExtensionDtype]]" and "List[str]") pandas/tests/dtypes/test_common.py:404: error: Unsupported operand types for + ("List[Union[Series, str, str, Any, ExtensionDtype, Any, ExtensionDtype]]" and "List[str]")

I'm not sure what to make of this, but I'm looking into trying to figure out what the Any might be so I can put it into the type annotation.

WillAyd · 2019-11-03T20:20:08Z

Does changing from List to Sequence help? Haven't reviewed thoroughly but the invariant nature of the List might be the issue

WillAyd · 2019-11-03T22:54:46Z

pandas/tests/dtypes/test_common.py

@@ -322,9 +326,13 @@ def test_is_datetimelike():
    assert com.is_datetimelike(s)


+integer_dtypes = []  # type: Sequence[Union[pd.Series, str, Dtype, ExtensionDtype]]


OK I think just type: List will be fine for these

WillAyd

lgtm. @simonjayhawkins care to take a look as well?

@HawkinsBA the CI failure is unrelated. If you merge master and re-push should go away

simonjayhawkins · 2019-11-07T00:38:22Z

lgtm. @simonjayhawkins care to take a look as well?

@WillAyd

noting @jbrockmendel comment regarding unpacking, which mypy accepts in an expression. I'm not convinced that this is more clear. Especially, since the use of generic List without type parameters is the same as List[Any] so does not add value to the type checking.

I know we have different opinions on the use of type: ignore but could potentially be used here and reference the mypy issue on list concatenation in expressions?

alternatively, perhaps assign all the values to the list in the initial assignment? could still use #type: List but would not need an expression concatenating items to an empty list. I think this would be clearer to a newcomer. Maybe use block capitals for the variable names. wdyt?

WillAyd · 2019-11-07T00:39:31Z

I don't have a strong preference; whatever works for me

simonjayhawkins · 2019-11-07T00:50:35Z

both these suggestions are effectively silencing mypy, whereas at least with unpacking the inferred type should have been more precise.

@jbrockmendel considering the above, are you against unpacking? This problem is likely to surface again?

WillAyd · 2019-11-07T00:56:40Z

Misinterpreted your earlier comment but I would prefer not to change to unpacking. These are just arguments to parameterize; we aren't mapping any kind of operations against the entire container so we don't need to fine-grain inference; either List or ignore would be OK I think

simonjayhawkins · 2019-11-07T00:58:22Z

I'm OK with just List but think the assignment should be in one statement for clarity.

…n-test-common-py

jbrockmendel · 2019-11-07T01:17:58Z

considering the above, are you against unpacking? This problem is likely to surface again?

If you're referring to the *args pattern I'd consider it near-last-resort, but not a strong enough opinion to hold up a PR if there is consensus.

simonjayhawkins · 2019-11-07T01:32:04Z

yeah, so I think we're agreed.

summarising since this (mypy) issue will crop up again...

the option that works for mypy, i.e. unpacking, means changing the code and is not newcomer friendly.

type:ignore may mask other issues

so refactoring out list concatenation in an expression into a variable and adding a variable annotation is the preferred approach.

simonjayhawkins

so refactoring out list concatenation in an expression into a variable and adding a variable annotation is the preferred approach.

it appears that to do this in one statement would require a cast...

integer_dtypes = (
    cast(List, [pd.Series([1, 2])])
    + ALL_INT_DTYPES
    + to_numpy_dtypes(ALL_INT_DTYPES)
    + ALL_EA_INT_DTYPES
    + to_ea_dtypes(ALL_EA_INT_DTYPES)
)


@pytest.mark.parametrize("dtype", integer_dtypes)

otherwise would still need two statements...

integer_dtypes = []  # type: List
integer_dtypes = (
    integer_dtypes
    + [pd.Series([1, 2])]
    + ALL_INT_DTYPES
    + to_numpy_dtypes(ALL_INT_DTYPES)
    + ALL_EA_INT_DTYPES
    + to_ea_dtypes(ALL_EA_INT_DTYPES)
)


@pytest.mark.parametrize("dtype", integer_dtypes)

@WillAyd given that this is still not much clearer, merge as is if happy.

WillAyd · 2019-11-07T16:22:48Z

Yea pretty tricky with these...I suppose the real intent is tough to realize from a static analysis. Curious to see how many we come across going forward

WillAyd · 2019-11-07T16:23:23Z

Thanks @HawkinsBA for the PR and @simonjayhawkins for review

Blake Hawkins added 2 commits October 19, 2019 13:55

Replacing list concatenation with "+"

b34fc3b

Update test_common.py

a266c69

Apply black formatting to test_common.py

jreback added the Typing type annotations, mypy/pyright type checking label Oct 23, 2019

WillAyd requested changes Oct 24, 2019

View reviewed changes

Update test_common.py

4d27ef2

Removed type(pd.Series[...])

HawkinsBA requested a review from WillAyd October 26, 2019 22:23

jbrockmendel and others added 21 commits October 28, 2019 19:13

BUG: Fix TypeError in _cython_agg_blocks (pandas-dev#29035)

c208bbe

CI: xfail on numpy 1.18 (pandas-dev#29057)

0e556c2

* xfail on numpy 1.18 * CI: try using numpy wheel

Stop catching TypeError in groupby methods (pandas-dev#29060)

2cfc88f

fixed issue of mypy for test_ujson (pandas-dev#29022)

5fbbe9e

CLN: derivation of nogil param (pandas-dev#29047)

5506b82

REF: remove groupby_helper (pandas-dev#29040)

ad01c8b

ENH: Informative dtype message for for assert_series_equal (pandas-de…

2d2f220

…v#28993)

TST: add regression test for all-none-groupby (pandas-dev#29067)

009ffc4

Closes pandas-dev#21624

Fix mypy errors for pandas\tests\*: test_convert_to.py (pandas-dev#28965

3940401

)

CLN: catch less in groupby (pandas-dev#29077)

b106ef3

CLN: simplify take_2d_multi (pandas-dev#29065)

429ed6e

REF: use fused types in algos_rank_helper (pandas-dev#29044)

ac4469c

DOC: updated categorical docstring (pandas-dev#29068)

1939942

CLN: tighten exception catching in indexes (pandas-dev#29078)

6ce48d9

[pandas-dev#22550] Remove TestData from series-tests test_quantile.py (…

b036967

…pandas-dev#29096)

comments, catch less (pandas-dev#29088)

ac45185

REF: remove algos_rank_helper (pandas-dev#29086)

6c93ef1

CLN: Exception in DataFrame._reduce (pandas-dev#29085)

a8cbd11

[pandas-dev#22550] Remove TestData from series-tests test_operators.py (

1f20f63

pandas-dev#29084)

TST: 2d index when constructing dataframe (pandas-dev#25416). (pandas…

d5f430a

…-dev#29083)

Blake Hawkins and others added 3 commits November 2, 2019 17:44

Updating type annotation in test_common.py

0dc0cc5

Remove unused import from test_common.py

05f3b16

Trying to fix Azure Pipelines

8d21fe0

Blake added 2 commits November 3, 2019 14:39

Replacing List with Sequence and removing Any

a3eea24

Applying black formatting

d01d5bb

WillAyd requested changes Nov 3, 2019

View reviewed changes

Blake Hawkins added 2 commits November 5, 2019 18:00

Replaciing type stubs with List

3f37553

Remove unused import from test_common.py

3ba0b50

WillAyd approved these changes Nov 7, 2019

View reviewed changes

Merge remote-tracking branch 'upstream/master' into Fix-mypy-errors-i…

1fe031d

…n-test-common-py

simonjayhawkins approved these changes Nov 7, 2019

View reviewed changes

WillAyd added this to the 1.0 milestone Nov 7, 2019

WillAyd merged commit b313f0c into pandas-dev:master Nov 7, 2019

simonjayhawkins mentioned this pull request Nov 7, 2019

Fix mypy errors for pandas\tests\* #28926

Closed

38 tasks

alimcmaster1 mentioned this pull request Nov 17, 2019

Fix mypy errors tests.dtypes.test_common #29110

Closed

5 tasks

Reksbril pushed a commit to Reksbril/pandas that referenced this pull request Nov 18, 2019

Fix mypy errors in testcommon py (pandas-dev#29179)

7e6d051

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

Fix mypy errors in testcommon py (pandas-dev#29179)

adcc9b5

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

Fix mypy errors in testcommon py (pandas-dev#29179)

768707a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix mypy errors in testcommon py #29179

Fix mypy errors in testcommon py #29179

HawkinsBA commented Oct 23, 2019 •

edited

Loading

WillAyd commented Oct 23, 2019

HawkinsBA commented Oct 23, 2019 •

edited

Loading

WillAyd left a comment

WillAyd Oct 24, 2019

HawkinsBA Oct 24, 2019

WillAyd Oct 24, 2019

HawkinsBA Oct 26, 2019

jbrockmendel commented Oct 28, 2019

pep8speaks commented Nov 3, 2019 •

edited

Loading

HawkinsBA commented Nov 3, 2019 •

edited

Loading

WillAyd commented Nov 3, 2019

WillAyd Nov 3, 2019

WillAyd left a comment

simonjayhawkins commented Nov 7, 2019

WillAyd commented Nov 7, 2019

simonjayhawkins commented Nov 7, 2019

WillAyd commented Nov 7, 2019

simonjayhawkins commented Nov 7, 2019

jbrockmendel commented Nov 7, 2019

simonjayhawkins commented Nov 7, 2019

simonjayhawkins left a comment

WillAyd commented Nov 7, 2019

WillAyd commented Nov 7, 2019

		@@ -322,9 +326,13 @@ def test_is_datetimelike():
		assert com.is_datetimelike(s)


		integer_dtypes = [] # type: Sequence[Union[pd.Series, str, Dtype, ExtensionDtype]]

Fix mypy errors in testcommon py #29179

Fix mypy errors in testcommon py #29179

Conversation

HawkinsBA commented Oct 23, 2019 • edited Loading

WillAyd commented Oct 23, 2019

HawkinsBA commented Oct 23, 2019 • edited Loading

WillAyd left a comment

Choose a reason for hiding this comment

WillAyd Oct 24, 2019

Choose a reason for hiding this comment

HawkinsBA Oct 24, 2019

Choose a reason for hiding this comment

WillAyd Oct 24, 2019

Choose a reason for hiding this comment

HawkinsBA Oct 26, 2019

Choose a reason for hiding this comment

jbrockmendel commented Oct 28, 2019

pep8speaks commented Nov 3, 2019 • edited Loading

Comment last updated at 2019-11-07 01:14:18 UTC

HawkinsBA commented Nov 3, 2019 • edited Loading

WillAyd commented Nov 3, 2019

WillAyd Nov 3, 2019

Choose a reason for hiding this comment

WillAyd left a comment

Choose a reason for hiding this comment

simonjayhawkins commented Nov 7, 2019

WillAyd commented Nov 7, 2019

simonjayhawkins commented Nov 7, 2019

WillAyd commented Nov 7, 2019

simonjayhawkins commented Nov 7, 2019

jbrockmendel commented Nov 7, 2019

simonjayhawkins commented Nov 7, 2019

simonjayhawkins left a comment

Choose a reason for hiding this comment

WillAyd commented Nov 7, 2019

WillAyd commented Nov 7, 2019

HawkinsBA commented Oct 23, 2019 •

edited

Loading

HawkinsBA commented Oct 23, 2019 •

edited

Loading

pep8speaks commented Nov 3, 2019 •

edited

Loading

HawkinsBA commented Nov 3, 2019 •

edited

Loading