CLN: Fixing mypy errors in pandas/conftest.py
State: closed

- [x] xref #28926 
- [x] 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 annotate these instead of ignoring? I think should can just use Dtype from pandas._typing. Might need to modify that to account for built in types

Sent from my iPhone

> On Oct 16, 2019, at 5:30 PM, Angela Ambroz <notifications@github.com> wrote:
> 
> ﻿
>  addresses, but does not close, #28926
>  tests added / passed
>  passes black pandas
>  passes git diff upstream/master -u -- "*.py" | flake8 --diff
>  whatsnew entry
> You can view, comment on, or merge this pull request online at:
> 
>   https://github.com/pandas-dev/pandas/pull/29046
> 
> Commit Summary
> 
> Ignoring some mypy errors related to imports and list inference
> File Changes
> 
> M pandas/conftest.py (10)
> Patch Links:
> 
> https://github.com/pandas-dev/pandas/pull/29046.patch
> https://github.com/pandas-dev/pandas/pull/29046.diff
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub, or unsubscribe.

Cool - hadn't seen pandas._typing. Will do.
Hello @angelaambroz! 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 2020-03-25 18:36:23 UTC
I've left the `# type: ignore` for the import statement, since I suspect it's related to [what the mypy docs describe](https://mypy.readthedocs.io/en/latest/running_mypy.html#how-mypy-handles-imports) when a module is present, but mypy is unable - for whatever reason - to check its type hints. 
You should encapsulate those in Type[...] otherwise this would be looking for an instance of those. 

FWIW I’d rather be explicit about these rather than relying on any hierarchy or inference. In the future we’d probably encapsulate in Literal

Sent from my iPhone

> On Oct 18, 2019, at 4:50 PM, Angela Ambroz <notifications@github.com> wrote:
> 
> ﻿
> @angelaambroz commented on this pull request.
> 
> In pandas/conftest.py:
> 
> > @@ -481,7 +482,7 @@ def tz_aware_fixture(request):
>  
>  UNSIGNED_INT_DTYPES = ["uint8", "uint16", "uint32", "uint64"]
>  UNSIGNED_EA_INT_DTYPES = ["UInt8", "UInt16", "UInt32", "UInt64"]
> -SIGNED_INT_DTYPES = [int, "int8", "int16", "int32", "int64"]
> +SIGNED_INT_DTYPES = [int, "int8", "int16", "int32", "int64"]  # type: Dtype
> Thanks, both. Just pushed what I think you both meant - let me know. I'm still learning about types! Looks like complex was also duck typed to float.
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or unsubscribe.

@angelaambroz thanks for the PR! Are you still working on this?
Eek, sorry, yes. I'd like to keep working on this. Apologies for the silence. 
CI fails on `pytest pandas/conftest.py`, due to `ExtensionDtype` not being recognized. It's currently only imported if `TYPE_CHECKING==True`. There's a comment warning against circular imports, so I'm hesitating moving
```
from pandas.core.dtypes.dtypes import ExtensionDtype  # noqa: F401
```
out of the conditional. Halp? 🆘 

@angelaambroz can you take care of the last comment, so we can finally merge this please? Please ping us once the CI is green, for a faster merge.
Argh, sorry, I'm traveling and away from a laptop until Jan 8. Can someone
else do the last push? Sorry it's been hanging so long.

On Sun, Dec 29, 2019, 3:03 PM Marc Garcia <notifications@github.com> wrote:

> @angelaambroz <https://github.com/angelaambroz> can you take care of the
> last comment, so we can finally merge this please? Please ping us once the
> CI is green, for a faster merge.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/29046?email_source=notifications&email_token=AAWJPII4O7ZCSMV5MV2CMX3Q3CU35A5CNFSM4JBSTLY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHZAFOA#issuecomment-569508536>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAWJPIJPHDLH5ZZRYN4MX7DQ3CU35ANCNFSM4JBSTLYQ>
> .
>

You can do the last commit from the website, there is a dropdown to accept the proposed change. I tried to do it myself, but looks like GitHub just let the author accept it.
Ack. CI failing linting - it doesn't like the import order in conftest. 
- I could try fixing this via the GitHub web UI (iffy, but I might be able to do it).
- Someone else picks this up.
- We wait until Jan 8/9, when I'm back at a computer and can debug properly.

> Ack. CI failing linting - it doesn't like the import order in conftest.
> 
> * I could try fixing this via the GitHub web UI (iffy, but I might be able to do it).
> * Someone else picks this up.
> * We wait until Jan 8/9, when I'm back at a computer and can debug properly.

You can run `isort -rc pandas` to fix the import order issue. For more info see here: https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#import-formatting
@angelaambroz can you fix up merge conflict? I think this is pretty much ready
It's failing on type checks now; this was unexpected, but I didn't dive deep into the merge conflicts on `pandas/_typing.py`, where I saw some changes (I intended just to change the `Dtype` type). I'll try to go through this later today and see what's up. 
@angelaambroz can you merge master
Hi @angelaambroz  - sorry to chase you up, just wanted to ask whether you're still working on this :)
@MarcoGorelli Feel free to grab it - I'm being very slow and not finding enough time to get back to this! I'd be happy to continue, but don't want to hold things up. Thanks for checking in!
> I'd be happy to continue, but don't want to hold things up

It's OK, I don't believe this is _too_ urgent, so feel free to continue and to ask for help if you're stuck
@angelaambroz any interest in fixing up the merge conflict and addressing last comment?
👋 Yes, here! I hang my head in shame. But I can do this later today. 
Thanks @angelaambroz for seeing this through