Skip to content

fixed bug in CenteredNorm, issue #19972 #19978

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 2 commits into from
May 5, 2021

Conversation

jpmattern
Copy link
Contributor

PR Summary

Fixed the CenteredNorm bug in issue #19972

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • 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).

@smartlixx
Copy link
Contributor

A test is needed IMHO.

Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

Yes, we should add a test here - there is code in the original issue (#19972) that should be fine.

@dstansby dstansby added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Apr 16, 2021
@dstansby dstansby added this to the v3.4.2 milestone Apr 16, 2021
@tacaswell
Copy link
Member

👍 This can go in on one review once a test is aded.

@jpmattern
Copy link
Contributor Author

Sorry, I was away for the weekend. Happy to add a test. Basically one that recreates the circumstances of the reported bug, I presume?

@jpmattern
Copy link
Contributor Author

While the reported bug (#19972) raised an exception, the underlying issue was that the coded tested for self.vmax is None rather than self._halfrange is None. So in the old code, a manually set halfrange would have been reset by a call to autoscale_None would have reset it. Fixed that bug and added a small test that the old code would not have passed for both reasons: the exception and the reset of autoscale_None.

@timhoffm
Copy link
Member

Ping @dstansby

@jklymak jklymak requested a review from dstansby April 23, 2021 14:40
@QuLogic QuLogic dismissed dstansby’s stale review May 5, 2021 00:13

Test added.

@QuLogic QuLogic merged commit 8e69c9b into matplotlib:master May 5, 2021
@tacaswell
Copy link
Member

@meeseeksbot backport to v3.4.x

@QuLogic
Copy link
Member

QuLogic commented May 5, 2021

@meeseeksdev backport to v3.4.x

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request May 5, 2021
QuLogic added a commit that referenced this pull request May 5, 2021
…978-on-v3.4.x

Backport PR #19978 on branch v3.4.x (fixed bug in CenteredNorm, issue #19972)
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.

6 participants