Fix mypy errors in testcommon py
State: closed

- [x] xref [#28926](https://github.com/pandas-dev/pandas/issues/28926)
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
Can you post the error this solves? Does annotating these instead of unpacking change anything?
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?
@HawkinsBA can you rebase
Hello @HawkinsBA! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



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

##### Comment last updated at 2019-11-07 01:14:18 UTC
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.
Does changing from `List` to `Sequence` help? Haven't reviewed thoroughly but the invariant nature of the List might be the issue
> 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?
I don't have a strong preference; whatever works for me
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? 
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

I'm OK with just `List` but think the assignment should be in one statement for clarity. 
> 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.
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.


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
Thanks @HawkinsBA for the PR and @simonjayhawkins for review