Fix type hints for undeprecated contour APIs by ksunden · Pull Request #27217 · matplotlib/matplotlib · GitHub
Skip to content

Fix type hints for undeprecated contour APIs - #27217

Merged
rcomer merged 1 commit into
matplotlib:mainfrom
ksunden:contour_type
Oct 28, 2023
Merged

Fix type hints for undeprecated contour APIs#27217
rcomer merged 1 commit into
matplotlib:mainfrom
ksunden:contour_type

Conversation

@ksunden

@ksunden ksunden commented Oct 27, 2023

Copy link
Copy Markdown
Member

PR summary

They had been in the limbo land of deprecated APIs that I didn't bother to ensure were the best type
hints because they were deprecated.
So I just added them to the allowlist for stubtest, but now that they are undeprecated,
make sure that they are correct.

While I'm here, may as well get rid of the allow list entries for all of them,
even the ones that are still deprecated, as that is documented standard practice going
forward that pyi match implementation for deprecations as well

PR checklist

@ksunden ksunden added this to the v3.8.1 milestone Oct 27, 2023
Comment thread lib/matplotlib/contour.pyi Outdated
Comment on lines +114 to +116

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I've read these correctly, these are lists of lists of arrays:

Suggested change
def allkinds(self) -> list[np.ndarray]: ...
@property
def allsegs(self) -> list[np.ndarray]: ...
def allkinds(self) -> list[list[np.ndarray]]: ...
@property
def allsegs(self) -> list[list[np.ndarray]]: ...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, and technically the source of these is Path.codes and Path.vertices, which actually are only typed as ArrayLike... and the former can actually just be None (though that is not reflected by the type hint) as well (which I saw when I just ran it to be sure it was actually nested lists)

It is one of these "in practice in this usage the type will be ndarray or None", but that is relying on knowledge outside of the type system... so I suspect if we were to inline the type hints it would cause a clash...

I also think that the None behavior is not super intentional in this context... There is a note in the Path docstring that says "don't use codes directly, use iter_segments so that you don't get the Nones from codes"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Only the empty path I think will actually have codes=None... but that is in fact a case covered by the code...

@ksunden

ksunden commented Oct 27, 2023

Copy link
Copy Markdown
Member Author

Noticed that tcolors (one of the still deprecated properties) is also missing a layer of nesting, for some reason it is a list of 1-tuples of 4-tuples...

They whad been in the limbo land of deprecated APIs that I didn't bother to ensure were the _best_ type hints because they were deprecated. So I just added them to the allowlist for stubtest, but now that they are undeprecated, make sure that they are correct. While I'm here, may as well get rid of the allow list entries for all of them, even the ones that are still deprecated, as that is documented standard practice going forward that pyi match implementation for deprecations as well
@QuLogic

QuLogic commented Oct 27, 2023

Copy link
Copy Markdown
Member

Yes, good catch.

@rcomer rcomer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm glad I am not the only one confused by tcolors.

@rcomer
rcomer merged commit 518b268 into matplotlib:main Oct 28, 2023
@lumberbot-app

lumberbot-app Bot commented Oct 28, 2023

Copy link
Copy Markdown

rcomer added a commit to rcomer/matplotlib that referenced this pull request Oct 28, 2023
timhoffm added a commit that referenced this pull request Oct 28, 2023
Backport PR #27217 on branch v3.8.x: Fix type hints for undeprecated contour APIs
@ksunden ksunden mentioned this pull request Nov 2, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants