-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TYP: Annotate plotting stacker #36016
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.
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
TYP: Annotate plotting stacker #36016
Conversation
@@ -150,7 +154,7 @@ def _make_plot(self): | |||
labels = [pprint_thing(key) for key in range(len(labels))] | |||
self._set_ticklabels(ax, labels) | |||
|
|||
def _set_ticklabels(self, ax, labels): | |||
def _set_ticklabels(self, ax: "Axes", labels): |
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've opened #36034 so that when reviewing we can just click on and get definition as doesn't work for string literals (in VS Code anyway)
pandas/plotting/_matplotlib/core.py
Outdated
@@ -667,10 +665,10 @@ def _plot(cls, ax, x, y, style=None, is_errorbar=False, **kwds): | |||
if style is not None: | |||
args = (x, y, style) | |||
else: | |||
args = (x, y) | |||
args = (x, y) # type:ignore |
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.
if adding ignores can you add the mypy error code and the mypy error message as a comment.
In this case, it's a relative simple cleanup. add a variable type declaration for args?
@@ -916,15 +914,15 @@ def __init__(self, data, x, y, **kwargs): | |||
self.y = y | |||
|
|||
@property | |||
def nseries(self): | |||
def nseries(self) -> int: |
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.
possibly a use of Literal after vendoring PR merged
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.
Thanks @jbrockmendel lgtm except 'bare' #type: ignore
…notate-plotting-stacker
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.
Thanks @jbrockmendel lgtm pending green
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
yea also lgtm test failures seems unrelated so restarted. merge on green |
Thanks @jbrockmendel |
In AreaPlot._plot we call
ax.fill_between
, which meansax
must be anAxes
object (and in particular, not anAxis
object). This chases down all the other places we can inferAxes
from that.Then some edits in an
__init__
to get mypy passing, and revert annotations of_plot
in a few places because mypy complained about signature mismatch.