Skip to content

TYP: libinterval #41059

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 20 commits into from
Closed

TYP: libinterval #41059

wants to merge 20 commits into from

Conversation

jbrockmendel
Copy link
Member

This one is pretty ugly.

@jreback jreback added Interval Interval data type Typing type annotations, mypy/pyright type checking labels Apr 20, 2021
@jreback jreback added this to the 1.3 milestone Apr 20, 2021
simonjayhawkins
simonjayhawkins reviewed Apr 24, 2021
View reviewed changes
@simonjayhawkins
Copy link
Member

This one is pretty ugly.

is something awry with mypy or do you know what changes here cause issues with the doc decorator.

simonjayhawkins
simonjayhawkins requested changes Apr 26, 2021
View reviewed changes
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe break off the io changes for now

@jbrockmendel
Copy link
Member Author

is something awry with mypy or do you know what changes here cause issues with the doc decorator.

no idea whats making mypy complain

@jbrockmendel
Copy link
Member Author

maybe break off the io changes for now

IIRC i did only what was necessary to get mypy to stop complaining about interval.pyi

simonjayhawkins
simonjayhawkins requested changes Apr 27, 2021
View reviewed changes
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe break off the io changes for now

IIRC i did only what was necessary to get mypy to stop complaining about interval.pyi

we can't merge this with all these error: Cannot determine type of ignores that have appeared. not sure what else to suggest.

@jbrockmendel jbrockmendel mentioned this pull request Apr 27, 2021
Merged
@jbrockmendel
Copy link
Member Author

we can't merge this with all these error: Cannot determine type of ignores that have appeared. not sure what else to suggest.

IIUC mypy's complaint is bc e.g. Index.memory_usage is decorated with @doc(IndexOpsMixin._memory_usage). doc is annotated with -> Callable[[F], F] which i would expect would prevent this problem

@jbrockmendel
Copy link
Member Author

@simonjayhawkins i think the reason why so many new mypy complaints are popping up is because Scalar no longer resolves to Any. will try to chip away at these

jreback
jreback requested changes May 17, 2021
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rebase. this is a lot of change, not sure if easier to do in pieces.

pandas/_typing.py
@@ -84,10 +86,10 @@

# scalars

PythonScalar = Union[str, int, float, bool]
PythonScalar = Optional[Union[str, int, float, complex, bool, date, time, timedelta]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of Optional, can you use type(None) to be more explict?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should just do a pass to modernize this entire file

@jbrockmendel
Copy link
Member Author

this is a lot of change, not sure if easier to do in pieces

Haven't found a good way to do this. bc this is the last thing that mypy doesnt recognize in pd._typing.Scalar, it means a bunch of things that currently resolve to Any are now recognized. I'll keep trying this periodically, but for the time being this is on the backburner.

simonjayhawkins
simonjayhawkins reviewed May 22, 2021
View reviewed changes
pandas/io/parsers/python_parser.py Outdated
@@ -1,3 +1,5 @@
from __future__ import annotations
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could do this and pandas/io/parsers/base_parser.py in a pre-cursor to reduce the noise here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

@simonjayhawkins
Copy link
Member

can pandas/_libs/tslibs/nattype.pyi or pandas/_libs/interval.pyi be added in seperate PRs.

also it is possible to partially type a stub so that could be get some of this in.

https://github.com/python/typeshed/blob/master/CONTRIBUTING.md#incomplete-stubs

@simonjayhawkins simonjayhawkins removed this from the 1.3 milestone May 24, 2021
jbrockmendel added a commit to jbrockmendel/pandas that referenced this pull request May 27, 2021
@jbrockmendel
TYP: io broken off from pandas-dev#41059
d90a3a6
@jbrockmendel
Copy link
Member Author

Mothballing to clear the queue, as there is no clear path forward here.

@jbrockmendel jbrockmendel added the Mothballed Temporarily-closed PR the author plans to return to label Jun 12, 2021
@simonjayhawkins simonjayhawkins mentioned this pull request Feb 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonjayhawkins simonjayhawkins simonjayhawkins requested changes

@jreback jreback jreback requested changes

Assignees
No one assigned
Labels
Interval Interval data type Mothballed Temporarily-closed PR the author plans to return to Typing type annotations, mypy/pyright type checking
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jbrockmendel @simonjayhawkins @jreback