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

Corrected plot_anomaly_log_colouring for new Matplotlib linscale rules. #4115

Merged
merged 1 commit into from
May 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/gallery_code/general/plot_anomaly_log_colouring.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ def main():
# Create a 'logarithmic' data normalization.
anom_norm = mcols.SymLogNorm(
linthresh=minimum_log_level,
linscale=1,
linscale=0.01,
vmin=-maximum_scale_level,
vmax=maximum_scale_level,
)
# Setting "linthresh=minimum_log_level" makes its non-logarithmic
# data range equal to our 'zero band'.
# Setting "linscale=1" maps the whole zero band to the middle colour value
# Setting "linscale=0.01" maps the whole zero band to the middle colour value
# (i.e., 0.5), which is the neutral point of a "diverging" style colormap.
Copy link
Member

@rcomer rcomer May 4, 2021

Choose a reason for hiding this comment

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

I'll be honest, I'm really struggling to match this comment up with what the mpl docs say about linscale. I'm getting the feeling that either the original comment was wrong, or the behaviour has changed. Or I'm just missing something... 😕

Copy link
Member

Choose a reason for hiding this comment

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

It's the first bit about "linthresh=minimum_log_level" that I'm particularly struggling with, rather than the changed bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The language of these comment lines makes no sense to me either, but I assumed that to be due to the writer's greater knowledge in this area. Seems important to understand and correct before merging.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like @pp-mo was the originator, but a Whole Seven Years Ago!!! #1040

Copy link
Member

Choose a reason for hiding this comment

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

Given how much matplotlib would have moved on in seven years, it's entirely possible that there are better ways of achieving this outcome now anyway. So possibly we should just merge this now for the sake of fixing master, and open a new issue to revisit later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having read in more detail and talked it over with @stephenworsley, I'm reasonably happy with how it's written already.

Copy link
Member

@pp-mo pp-mo Jul 6, 2021

Choose a reason for hiding this comment

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

Just found this.
FWIW there was a perfectly good documentation for this up to Matplotlib v3.3,
https://matplotlib.org/3.3.4/api/_as_gen/matplotlib.colors.SymLogNorm.html

But they seem to have lost it somewhere since ??

update : It's not unique : a lot of the classes have lost their docstring details.
I'm trying to work out how to raise this as an issue.


# Create an Axes, specifying the map projection.
Expand Down
5 changes: 2 additions & 3 deletions lib/iris/tests/results/imagerepo.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@
"gallery_tests.test_plot_anomaly_log_colouring.TestAnomalyLogColouring.test_plot_anomaly_log_colouring.0": [
"https://scitools.github.io/test-iris-imagehash/images/v4/ec4464e185a39f93931e9b1e91696d2949dde6e63e26a47a5ad391938d9a5a0c.png",
"https://scitools.github.io/test-iris-imagehash/images/v4/ecc164e78e979b19b3789b0885a564a56cc2c65e3ec69469db1bdb9a853c1e24.png",
"https://scitools.github.io/test-iris-imagehash/images/v4/ece164e68e979b19b3781b0885a564a56ccac65e3ec69469db1bdb9a853c1e24.png",
"https://scitools.github.io/test-iris-imagehash/images/v4/ece164e796979a1b39781b2881a564a56ccac6da3e87947bcb1bdb9a843c1e24.png"
"https://scitools.github.io/test-iris-imagehash/images/v4/ece164e68e979b19b3781b0885a564a56ccac65e3ec69469db1bdb9a853c1e24.png"
],
"gallery_tests.test_plot_atlantic_profiles.TestAtlanticProfiles.test_plot_atlantic_profiles.0": [
"https://scitools.github.io/test-iris-imagehash/images/v4/9f8260536bd28e1320739437b5f437b0a51d66f4cc5d08fcd00fdb1c93fcb21c.png",
Expand Down Expand Up @@ -1048,4 +1047,4 @@
"https://scitools.github.io/test-iris-imagehash/images/v4/82fe81987fdf77ffe0002addd4002805dd28df67d9a9d4625bfddc209841de20.png",
"https://scitools.github.io/test-iris-imagehash/images/v4/82fa80997f547799a0037a00d52f0956ddaf9f7e98a1816e09f5d8260bfffe00.png"
]
}
}