Skip to content

Small cleanup to VertexSelector. - #23403

Merged
QuLogic merged 1 commit into
matplotlib:mainfrom
anntzer:vsp
Jul 11, 2022
Merged

QuLogic merged 1 commit into
matplotlib:mainfrom
anntzer:vsp

Conversation

@anntzer

@anntzer anntzer commented Jul 8, 2022

Copy link
Copy Markdown
Contributor

Make the class picklable, by making canvas a property and using a
picklable callback connect. Also some docstring fixes.

PR Summary

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

Make the class picklable, by making `canvas` a property and using a
picklable callback connect.  Also some docstring fixes.
Comment thread lib/matplotlib/lines.py
'pick_event', self.onpick)
self.ind = set()

canvas = property(lambda self: self.axes.figure.canvas)

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.

Was there a reason we needed canvas in the first place?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would not have made this public to start with, but e.g. the HighlightSelector example (in the docstring) directly accesses it and it is marginally more convenient to have it there, so let's not bother with deprecating it?

@timhoffm timhoffm 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.

canvas is de-facto public. Changing this to a property does not make it worse. We can always decide later whether we want to deprecate it.

@QuLogic QuLogic added this to the v3.6.0 milestone Jul 11, 2022
@QuLogic
QuLogic merged commit fe870f5 into matplotlib:main Jul 11, 2022
@anntzer
anntzer deleted the vsp branch July 11, 2022 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants