Skip to content

TYP: annotate plotting._matplotlib.tools #35968

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

Merged

Conversation

jbrockmendel
Copy link
Member

Same idea as #35960, focused on clarifying Axis vs Axes

jbrockmendel and others added 22 commits August 20, 2020 21:19
@@ -340,15 +351,21 @@ def _handle_shared_axes(axarr, nplots, naxes, nrows, ncols, sharex, sharey):
_remove_labels_from_axis(ax.yaxis)


def _flatten(axes):
def _flatten(axes: Union["Axes", Sequence["Axes"]]) -> Sequence["Axes"]:
Copy link
Member

Choose a reason for hiding this comment

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

return is 1d np.ndarray? axes can also be (np.ndarray, IndexClass)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Index.ravel returns an ndarray, so we always get ndarray here

Copy link
Member

Choose a reason for hiding this comment

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

can you update return type

Copy link
Member Author

Choose a reason for hiding this comment

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

to what?

Copy link
Member

Choose a reason for hiding this comment

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

np.ndarray?

Copy link
Member Author

Choose a reason for hiding this comment

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

If there were a way to indicate ndarray["Axes"] I'd agree. until then, it is the "Axes" that is most relevant

Copy link
Member

Choose a reason for hiding this comment

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

so mypy is green since np.ndarray resolves to Any, so Any is compatible with Sequence["Axes"].

I guess that we could add result = cast(Sequence["Axes"], result) when numpy types are available.

@simonjayhawkins simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Aug 30, 2020
@simonjayhawkins simonjayhawkins added this to the 1.2 milestone Aug 30, 2020
@simonjayhawkins simonjayhawkins merged commit 0e621bc into pandas-dev:master Aug 31, 2020
@simonjayhawkins
Copy link
Member

Thanks @jbrockmendel

@jbrockmendel jbrockmendel deleted the annotate-plotting-tools branch August 31, 2020 14:45
jbrockmendel added a commit to jbrockmendel/pandas that referenced this pull request Aug 31, 2020
kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants