Gallery: update seasonal ensemble example by rcomer · Pull Request #3933 · SciTools/iris

rcomer · 2020-11-29T20:44:35Z

🚀 Pull Request

Description

Iris has moved on a lot since many of the gallery examples were added. This PR simplifies the seasonal forecast example as well as dropping the old school "%s" string formatting.

Consult Iris pull request check list

rcomer · 2020-11-29T20:45:39Z

 # coordinate, adding appropriate lagged ensemble metadata
 surface_temp = iris.load_cube(
 iris.sample_data_path("GloSea4", "ensemble_???.pp"),
- iris.Constraint("surface_temperature", realization=lambda value: True),


I don't actually understand the realization part of this constraint but removing it didn't make any difference to the plot.

Wow. Okay, what the heck... hmmm without digging this seems like an ancient workaround for something.
The only thing I can think of is that it's ensuring that the realization coordinate is always present? Dunno... anyways if removing it makes no difference, then let's remove it. Great spot 👀

OK, because I hate not knowing, I had a play:
If I don't use the callback or the realization constraint, I get a MergeError because of mismatched "realization" coordinates, as we might expect.
If I don't use the callback but reinstate the realization constraint, the plot works fine but more maps and lines are missing. So yeah, it seems this is a "coordinate exists" constraint, which actually seems quite useful, though I guess you could get the same effect using a cube_func.

@rcomer Well at the very least it deserves a comment to explain what the heck it's doing.
It's a neat pattern to employ without cracking open the cube_func. So let's honour the original authors intent but with some clarification for the reader... what say ye? 🤔

Definitely: if we're going to include it, it should be explained! But... if we advertise this behaviour, should there also be a test for this behaviour? Is there a test for this behaviour? If not, should I add one, and if so where?
And I thought a gallery update would be straightforward 🙄 🤣

No worries. Let's conquer and divide here 💪
How's about you add a comment, if you're happy doing that and I'll add some appropriate test coverage, and target a pull-request on your rcomer:gallery-seasonal-ensemble branch.
Deal?
(Ode to a simple life 🙏)

Yeah sounds good to me.

I figured it's easier to explain the constraint if it's defined on its own line.

rcomer · 2020-11-29T20:52:15Z

-
- # Subsetting a circular longitude coordinate always results in a circular
- # coordinate, so set the coordinate to be non-circular
- nino_cube.coord("longitude").circular = False


Not really sure why this mattered...?

Hmm I strongly suspect it is to do with how the plot was rendered around/over the dateline,

But then we're taking an area mean before we plot it... 🤷

bjlittle · 2020-12-03T07:52:12Z

- % (time.strftime("%B %Y"),)
- )
+ title = "Surface temperature ensemble forecasts for {}"
+ plt.suptitle(title.format(time.strftime("%B %Y")))


@rcomer Even better, use an f-string here? i.e.,
plt.suptitle(f"Surface temperature ensemble forecasts for {time.strftime('%B %Y')}")

Ooooo I've never used an f-string before 😁

Just as format is a step-up from %, f-strings are a step-up from format IMHO
e.g., generally once you've tried them, you don't look back. I kinda love'em now.

rcomer

Hi @bjlittle, I think I've covered those points now. I also, somewhat belatedly, realised that the comments were a bit inconsistent. So have been through and enforced capital letters and full stops.

rcomer · 2020-12-05T11:51:22Z

 # coordinate, adding appropriate lagged ensemble metadata
 surface_temp = iris.load_cube(
 iris.sample_data_path("GloSea4", "ensemble_???.pp"),
- iris.Constraint("surface_temperature", realization=lambda value: True),


I figured it's easier to explain the constraint if it's defined on its own line.

rcomer · 2020-12-05T11:53:38Z

+ # Set a global title for the postage stamps with the date formated by
+ # "monthname year".
+ time_string = time.strftime("%B %Y")
+ plt.suptitle(f"Surface temperature ensemble forecasts for {time_string}")


I thought it might make it more obvious what's going on if the time_string is defined before creating the f-string.

bjlittle

@rcomer LGTM 👍

Thanks for sticking with it 😄

rcomer · 2020-12-07T11:56:10Z

Thanks @bjlittle 🎉

* master: (23 commits) Added text to state the Python version used to build the docs. (SciTools#3989) add nox session conda list (SciTools#3990) Add abstract cube summary (SciTools#3987) automate docs discovery of iris and python versions (SciTools#3981) corrected syntax (SciTools#3980) core dev whatsnew entry (SciTools#3978) moved docs dir and updated references to it (SciTools#3975) Fix test_incompatible_dimensions test (SciTools#3977) remove explicit URLs for core dev names from latest.rst (SciTools#3973) document that iris.coords.Coord is an ABC (SciTools#3971) reorganise docs common links + add core devs (SciTools#3972) Docs whatsnew add dropdowns to the template (SciTools#3969) Docs whatsnew enumerated lists (SciTools#3970) Merge back v3p0p1 (SciTools#3966) Captilise installation heading - align SciTools#3958 content with SciTools#3940. (SciTools#3963) Merge back v3p0p0 (SciTools#3960) Extended the installation description (SciTools#3958) Put cube data on the x axis if plotting just a cube against a vertical or y coordinate (SciTools#3906) remove stock_mdi_arrays.npz (SciTools#3913) Gallery: update seasonal ensemble example (SciTools#3933) ...

rcomer commented Nov 29, 2020
View reviewed changes

rcomer added the Type: Documentation label Nov 29, 2020

rcomer commented Nov 29, 2020
View reviewed changes

Comment threaddocs/iris/gallery_code/meteorology/plot_lagged_ensemble.py Outdated

BryanLaura approved these changes Dec 1, 2020
View reviewed changes

bjlittle reviewed Dec 3, 2020
View reviewed changes

bjlittle self-assigned this Dec 3, 2020

bjlittle added this to the v3.1.0 milestone Dec 3, 2020

rcomer commented Dec 5, 2020
View reviewed changes

rcomer added 5 commits December 7, 2020 11:31

update seasonal example
c046ffd

add whatsnew
97b3150

have matplotlib do contour choosing work
080c86a

remove redundant import
e632a94

review changes and grammar
060e82a

rcomer force-pushed the gallery-seasonal-ensemble branch from 5cbe12d to 060e82aCompare December 7, 2020 11:34

bjlittle approved these changes Dec 7, 2020
View reviewed changes

bjlittle merged commit 3483482 into SciTools:masterDec 7, 2020

rcomer deleted the gallery-seasonal-ensemble branch December 7, 2020 11:59

rcomer mentioned this pull request Nov 1, 2022
Test for 'coord exists' constraint #5043
Merged

Uh oh!

Conversation

rcomer commented Nov 29, 2020

🚀 Pull Request

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rcomer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rcomerDec 5, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjlittle left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rcomer commented Dec 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rcomerDec 5, 2020•
edited
Loading

bjlittle left a comment •
edited
Loading