Add new Psyplot diagnostic

[schlunma]

Description

This PR adds a recipe and diagnostic that provides a high-level interface for the Psyplot package. This package can produce a large variety of plots, e.g., plot unstructured UGRID and ICON data:

Links to documentation:

• https://esmvaltool--2653.org.readthedocs.build/en/2653/recipes/recipe_psyplot.html
• https://esmvaltool--2653.org.readthedocs.build/en/2653/api/esmvaltool.diag_scripts.psyplot_diag.html#api-esmvaltool-diag-scripts-psyplot-diag
• https://esmvaltool--2653.org.readthedocs.build/en/2653/faq.html#can-esmvaltool-plot-arbitrary-model-output

@lukruh Would you be able to briefly test this recipe and approve the changes if you like them? I think this diagnostic might be especially interesting to you since it allows plotting arbitrary ICON output on the native grid. Maybe you could even try to plot native ICON output with it? Cheers!

---

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.

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

New or updated recipe/diagnostic

• 🧪 Recipe runs successfully
• 🧪 Recipe is well documented
• 🧪 Figure(s) and data look as expected from literature
• 🛠 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]

FYI: I removed this page since it contains no useful information. In the current doc it looks like this:

[valeriupredoi]

nice one, Manu! I am guessing this has nothing to do with PSY eh? 😁 - two minor comments from me, and a quick request to turn on then off one of the Test Github Actions to see if all's good with the env after adding those psy packages 👍

[valeriupredoi]

ah also - would this not make for a nice preprocessor? Not now - when we'll have plotting preprocessors - but just to keep it in mind, that is

[schlunma]

ah also - would this not make for a nice preprocessor? Not now - when we'll have plotting preprocessors - but just to keep it in mind, that is

ah also - would this not make for a nice preprocessor? Not now - when we'll have plotting preprocessors - but just to keep it in mind, that is

Definitely!! Plotting preprocessors is actually something we should discuss at the workshop as part of the long-term strategy 👍

[schlunma]

All GH action tests ran successful 🎉

[schlunma]

Thanks for the reviews @valeriupredoi @lukruh 🚀

[zklaus]

The test introduced in this PR produced an error in the nightly tests (see here). The problem seems to be a race condition where the package finds that it has no config file and tries to create that, but two processes try that at the same time and then can interfere. Upstream may be unwilling to address this because it is not likely a real life problem; we might mitigate it by creating the config before running the test.

[schlunma]

I didn't add a test in this PR...

I'm not really sure which test actually fails, the error message just says ERROR collecting esmvaltool/diag_scripts/psyplot_diag.py. Do we have a test that just imports diagnostics and checks if that works?

[zklaus]

Hm. Good point. I didn't read the message carefully enough. @bouweandela, @valeriupredoi any ideas?

[valeriupredoi]

having a looksee at this right now, I smell gagnam-style coding on the part of Psy

[zklaus]

I don't think the psyplot code is very bad. As @schlunma pointed out, it looks like pytest is searching for tests also in esmvaltool, even though we have separated them out in tests. This may be relevant.

[valeriupredoi]

Do we have a test that just imports diagnostics and checks if that works?

pytest does that - checks all imports in all recognized python scripts - it's actually a very useful feature since sometimes one doesn't have tests per se, but can just run a pytest instance in the CI to check if all imports work

[valeriupredoi]

I don't think the psyplot code is very bad. As @schlunma pointed out, it looks like pytest is searching for tests also in esmvaltool, even though we have separated them out in tests. This may be relevant.

ah yeah, was just saying in my previous comment 😁

[zklaus]

I don't think we should abuse Pytest's discovery logic as a poor man's import test.

[schlunma]

So how can we solve this? Adding a conftest.py file as suggeted here?

[bouweandela]

@zklaus We use pytest to run flake8 and doctests, that's the kind of tests it will find in the esmvaltool directory.

[valeriupredoi]

the problem is I can not manage to reproduce the fail on Linux under any circumstance: running five instance of a pytest (that only imports that module) in parallel with ./parallel_commands "pytest tests/unit/test_psy.py" "pytest tests/unit/test_psy_2.py" "pytest tests/unit/test_psy_2.py" "pytest tests/unit/test_psy.py" "pytest tests/unit/test_psy.py" where that parallel_commands actually runs each of the scripts on a separate thread, running with pytest -n 8 tests/unit/test_psy.py tests/unit/test_psy_2.py as well (many times too, gather some statistic) - can't hit that race issue even if you ask me to! OSX issue if you asked me

[bouweandela]

The problem seems to be a race condition where the package finds that it has no config file and tries to create that, but two processes try that at the same time and then can interfere. Upstream may be unwilling to address this because it is not likely a real life problem; we might mitigate it by creating the config before running the test.

It should be possible to use a library from different programs simultaneously. I would suggest reporting the issue and if the authors are indeed not interested in getting itt fixed we can think about workarounds.

[valeriupredoi]

I agree with Bouwe - I am trying to see if it's an OSX specific issue now, so we can noarrow down the parameter space for the Psy devvs, a bit hard coz it's like fishing on a frozen lake, not having a Mac myself

[zklaus]

Alright, sounds good. In the meantime, we just accept the occasional red cross on osx?

[valeriupredoi]

yeah we can change the sticker on the main README to point only to the Linux tests I think, so we save face for new users 😁

[valeriupredoi]

OK I managed to force the race condition to come from an actual test https://github.com/ESMValGroup/ESMValTool/runs/6470528634?check_suite_focus=true - again on OSX only, and it clearly is a race condition, probably due to how slow the execution is on Darwin machines on GA. Here is a suggestion for the Psy guys https://stackoverflow.com/questions/1586648/race-condition-creating-folder-in-python - @schlunma you wanna do the nonors to open an issue on the Psy GH? 🍺

[schlunma]

See psyplot/psyplot#52

[schlunma]

I already got an answer to the issue with a PR and was asked to check if that fix solves our problem. Any suggestions on how to do that? I also don't own a Mac, and as far as I can tell this only happens on OSX 😅

psyplot/psyplot#53

[zklaus]

@valeriupredoi, did you have a reproducible way to trigger the error? Otherwise I would say that the fix looks appropriate and explain that we have the issue only in a race condition that occurs rarely and that we'll get back if it doesn't solve the problem.

[valeriupredoi]

@zklaus not an 100% reproducible case, it looks to be 1 out of 6 or 9 times reproducible, so it's best to go the way you suggest 👍

[valeriupredoi]

BTW the psy thingie failed for Linux too - so it's not just OSX - it's a stock race condition, that happens everytime a magpie craps on a BMW 🐦