Skip to content

Fix axis inversion with loglocator and logitlocator. #14624

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
merged 1 commit into from
Aug 20, 2019

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jun 25, 2019

PR Summary

Reverts a minor part of #13593 to provide a different fix to #14615 and #14162

I chose to make nonsingular always order its arguments (and thus just change the behavior in the base class, which was only introduced in 3.1, rather than change the behavior of the subclass, which was only introduced in #13593 and not released yet) as that seems more likely to be respected by third-party implementations. On the other hand in #14643 I realized we can't really trust (for now) whether third-party implementations (or our own) do this ordering, so redoing a sort after-the-fact is likely the most robust solution...

The 3.1.x version is at #14623; this PR should not be backported.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 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.2.0 milestone Jun 25, 2019
@anntzer anntzer force-pushed the loglocatororder branch 3 times, most recently from 177164d to eec8dba Compare June 28, 2019 08:08
@anntzer anntzer added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Aug 6, 2019
@anntzer
Copy link
Contributor Author

anntzer commented Aug 6, 2019

release critical: a similar fix already made it to 3.1 in #14623.

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Technically correct. The argument for making nonsingular order the values sounds reasonable, but I don't have a complete overview of the consequences.

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.

modulo CI passing on rebase.

Anyone can merge on clean CI

@tacaswell tacaswell merged commit 5e3bede into matplotlib:master Aug 20, 2019
@anntzer anntzer deleted the loglocatororder branch August 20, 2019 14:51
@anntzer anntzer mentioned this pull request Aug 26, 2019
6 tasks
tacaswell added a commit to jb-leger/matplotlib that referenced this pull request Sep 4, 2019
In matplotlib#14624 we reverted
changing the order of the limits to match the input order.  Doing the
same here for consistency.
tacaswell added a commit to jb-leger/matplotlib that referenced this pull request Sep 6, 2019
In matplotlib#14624 we reverted
changing the order of the limits to match the input order.  Doing the
same here for consistency.

Sort the limits before comparing.  We want to make sure that it works
with the values reversed, but do not expect non-singular to preserve
the order.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants