Skip to content
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

Maintain artist addition order in Axes.mouseover_set. #11287

Merged
merged 1 commit into from
Jul 1, 2018

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented May 22, 2018

PR Summary

Fixes #11284, see discussion there.
Perhaps deprecate mouseover_set? Edit: yes.
Perhaps a test would be nice.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer anntzer added this to the v3.0 milestone May 22, 2018
@tacaswell
Copy link
Member

I have a slight concern that this is in a hot-loop on mouse-move so a slight preference for deprecating mouseover_set and replacing it with a (private?) list. At least we should use the private version of this set in backend bases.

@anntzer
Copy link
Contributor Author

anntzer commented May 23, 2018

Deprecated the public version, used the private version internally. As _OrderedSet.__iter__ just calls iter(self._od) (rather than yield from self._od), that will instantiate the native iterator on the OrderedDict instead of requiring going through an additional Python frame at every iteration.

Maintaining a list is also possible but it's just a very minor pain to reimplement add to check whether the item was already present or not, as well as whether discard should ignore missing elements (as set.discard does, but list.remove doesn't) (yes, that shouldn't happen as long as the state is internally consistent). It's not hard but feel free to write a PR if you want :p

@tacaswell
Copy link
Member

@anntzer rebased and force-pushed.

Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

Anyone (including @anntzer ) can merge this if CI passes.

@anntzer
Copy link
Contributor Author

anntzer commented Jul 1, 2018

Self-merging per #11287 (review).

@anntzer anntzer merged commit ead2ff8 into matplotlib:master Jul 1, 2018
@anntzer anntzer deleted the mouseoverset branch July 1, 2018 07:27
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.

imshow of multiple images produces old pixel values printed in status bar
3 participants