FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Move empty hexbin fix to make_norm_from_scale. by anntzer · Pull Request #24259 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Move empty hexbin fix to make_norm_from_scale. - #24259

Merged
QuLogic merged 1 commit into
matplotlib:mainfrom
anntzer:he
Oct 31, 2022
ConversationCommits (1)ChecksFiles changed
Merged

Move empty hexbin fix to make_norm_from_scale.#24259
QuLogic merged 1 commit into
matplotlib:mainfrom
anntzer:he

Conversation

anntzer commented Oct 23, 2022

Copy link
Copy Markdown
Contributor

3d2ffef added special-casing to handle empty log-scale hexbins, but the more back-compatible fix is to switch make_norm_from_scale to reproduce the old behavior of setting vmin/vmax to np.ma.masked when autoscaling with an empty array. (Whether that behavior is really better is unclear, but until this is properly investigated, it seems safer to go back to it.)

See #23944, #23922.

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).

anntzer added this to the v3.6.2 milestone Oct 23, 2022
jklymak reviewed Oct 23, 2022
View reviewed changes
Comment thread lib/matplotlib/colors.py Outdated
def autoscale(self, A):
# i.e. A[np.isfinite(...)], but also for non-array A's
in_trf_domain = np.extract(np.isfinite(self._trf.transform(A)), A)
if not in_trf_domain.size:

jklymak Oct 23, 2022 •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This changes this for everything that uses autoscale; is that OK? I guess its not failing any tests, but our masked tests were not terribly complete.

I find not in_trf_domain.size enigmatic - I assume it is None if there was nothing to extract? Maybe add a comment...

anntzer Oct 25, 2022

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

is that OK?

AFAICT this simply reverts to the pre 3.6 behavior, which had been accidentally changed.

not in_trf_domain.size

Sure, changed.

Move empty hexbin fix to make_norm_from_scale.
847e595 
3d2ffef added special-casing to handle empty log-scale hexbins, but the
more back-compatible fix is to switch make_norm_from_scale to reproduce
the old behavior of setting vmin/vmax to np.ma.masked when autoscaling
with an empty array.  (Whether that behavior is really better is
unclear, but until this is properly investigated, it seems safer to go
back to it.)

jklymak commented Oct 30, 2022

Copy link
Copy Markdown
Member

I do wonder about our continued support of np.masked. Does anyone talk about these kinds of things at scipy? Is np.masked dead, or not?

anntzer commented Oct 30, 2022

Copy link
Copy Markdown
Contributor Author

I personally don't care much about masked arrays, but IIRC @efiring was a proponent (sorry if I got this wrong).

QuLogic merged commit 6634d2a into matplotlib:main Oct 31, 2022
meeseeksmachine mentioned this pull request Oct 31, 2022
Merged
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Oct 31, 2022
Backport PR matplotlib#24259: Move empty hexbin fix to make_norm_from…
0af7ef5 
…_scale.
anntzer deleted the he branch October 31, 2022 23:34
tacaswell added a commit that referenced this pull request Nov 1, 2022
Merge pull request #24324 from meeseeksmachine/auto-backport-of-pr-24…
cc76eb6 
…259-on-v3.6.x

Backport PR #24259 on branch v3.6.x (Move empty hexbin fix to make_norm_from_scale.)
anntzer mentioned this pull request Nov 15, 2022
Merged
7 tasks
ksunden mentioned this pull request Feb 20, 2023
Merged
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

QuLogic QuLogic approved these changes
jklymak jklymak approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v3.6.2

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL