Use `transform_first=True` for contourf plots with Robinson projection to avoid cartopy bug

schlunma commented 1 month ago

Description

See https://github.com/SciTools/cartopy/issues/2457: using the Robinson projection with contourf may lead to very weird and wrong plots. This PR fixes that by setting transform_first=True as suggested here.

Related to https://github.com/SciTools/cartopy/issues/2457
- *

Before you get started

[x] ☝ Create an issue to discuss what you are going to do

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.

[x] 🛠 This pull request has a descriptive title
[x] 🛠 Code is written according to the code quality guidelines
[x] 🛠 Documentation is available
[x] 🛠 Tests run successfully
[x] 🛠 The list of authors is up to date
[x] 🛠 Any changed dependencies have been added or removed correctly
[x] 🛠 All checks below this pull request were successful

New or updated recipe/diagnostic

[x] 🧪 Recipe runs successfully
[x] 🧪 Recipe is well documented
[x] 🧪 Figure(s) and data look as expected from literature
[x] 🛠 Provenance information has been added

To help with the number of pull requests:

🙏 We kindly ask you to review two other open pull requests in this repository

schlunma commented 3 weeks ago

@esmvalbot please run monitor/recipe_monitor_with_refs.yml

esmvalbot[bot] commented 3 weeks ago

Since @schlunma asked, ESMValBot will run recipe monitor/recipe_monitor_with_refs.yml as soon as possible, output will be generated here

esmvalbot[bot] commented 3 weeks ago

ESMValBot is sorry to report it failed to run recipe monitor/recipe_monitor_with_refs.yml: exit is 1, output has been generated here

valeriupredoi commented 3 weeks ago

Bot says recipe monitor runs fine too https://github.com/ESMValGroup/ESMValTool/pull/3791#issuecomment-2445008624 - figured out what the bot's problem was (in private on Slack :rofl: )

valeriupredoi commented 3 weeks ago

@esmvalbot please run monitor/recipe_monitor_with_refs.yml

esmvalbot[bot] commented 3 weeks ago

Since @valeriupredoi asked, ESMValBot will run recipe monitor/recipe_monitor_with_refs.yml as soon as possible, output will be generated here

esmvalbot[bot] commented 3 weeks ago

ESMValBot is happy to report recipe monitor/recipe_monitor_with_refs.yml ran OK, output has been generated here

valeriupredoi commented 3 weeks ago

ESMValBot is happy to report recipe monitor/recipe_monitor_with_refs.yml ran OK, output has been generated here

you smol bundle of joy of a bot :robot:

schlunma commented 3 weeks ago

Good points! AFAIK, this is only relevant for map contouf plots (e.g., not for zonal mean profiles). I found the bug for the diagnostics that I changed here, but these are probably not the only ones affected. However, without deeper knowledge of the individual diagnostics it's really not easy to find out which are affected and which not.

My suggestion would be to either merge this now and be aware of that issue, or add this keyword to all mentions of contourf in our code (not sure if that might lead to other problems though...).

bettina-gier commented 3 weeks ago

I agree on not changing diagnostics we don't really know about and merging this pull request first for an immediate fix.

However, this isn't the only time something like this happens, would it be a good idea to include an "errata" page, maybe in the docs, where, potentially grouped by releases, we collect these 'found this error but not sure if anything else is affected' issues are collected. Then during release testing we can link to that page for diagnostic maintainers as a first step to check if that is one of the issues when their recipe fails. I've often seen people during the release testing saying "hey this issue was already reported here insert link", when it might be good to just have a list of it to begin with.

schlunma commented 3 weeks ago

In principle I like this idea, but this will be hard to maintain. It will be very easy to forget to add things, remove outdated things, etc.

In a way this is already documented in this PR here, right? So if people search the repository before reporting new problems (which people should always do!), this will be found. I am gonna merge this now to fix the diags; but we can of course further discuss this.

bettina-gier commented 3 weeks ago

hmm that's why I wanted it in sections of "found after release x before release y". Maybe docs is a bad place, a pinned issue which links to the issues with a disclaimer of not being updated beyond adding the links

Edit: maybe a point for the workshop as "dissemination of information"

schlunma commented 3 weeks ago

Found the first problem with this: https://github.com/SciTools/cartopy/issues/2468 🙈

Luckily, the fix is easy: https://github.com/ESMValGroup/ESMValTool/pull/3797

ESMValGroup / ESMValTool