Removed netCDF4 package from integration tests of fixes

[schlunma]

This PR removes the use of the package netCDF4 in test for fixes to avoid segmentation faults.

• Addresses Segmentation fault when running tests on CircleCI #644 (I don't know if it closes this issue completely)

---

Before you get started

• ☝ Create an issue to discuss what you are going to do

Checklist

• PR has a descriptive title for the changelog
• Code follows the style guide
• Documentation is available for new functionality
• YAML files pass pre-commit or yamllint checks
• Circle/CI tests pass
• Codacy code quality checks pass
• Documentation builds successfully on readthedocs
• Unit tests are available

---

To help with the number pull requests:

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

[valeriupredoi]

dat was superqwik, cheers Manu! I am having a look and stress test now 👍

[valeriupredoi]

good job with the PR @schlunma 👍 🍺

• the PR helps with Segmentation fault when running tests on CircleCI #644 and makes things cleaner but it doesn't solve Segmentation fault when running tests on CircleCI #644 because of this comment and this comment although the PR does narrow down the problem and makes sure there are no stray thread-unsafe issues from netCDF4 in the tests, since yes, netCDF4 is thread-unsafe
• minor comment: there is one more import of netCDF4 in tests/integration/cmor/_fixes/sample_data/create_sample_data.py - do we want that changed too?

I reckon this is good to have and we need to move on understanding the iris-dask animosity from #644 but that's not the scope of this PR 👍

@bouweandela or @stefsmeets or @jvegasbsc one of you guys wanna have a look to so we have three pairs of eyes? If not good to go, Manu!

[schlunma]

Cheers V, thanks for reviewing! 🍺

* minor comment: there is one more import of `netCDF4` in `tests/integration/cmor/_fixes/sample_data/create_sample_data.py` - do we want that changed too?

Nope, we can't get rid of that since those (faulty) data cannot be produced with iris. However, this should not matter since this script is not used during the test. I only used it to create the sample data and I thought it would be good to keep it in the repo 👍

[valeriupredoi]

Cheers V, thanks for reviewing! beer

* minor comment: there is one more import of `netCDF4` in `tests/integration/cmor/_fixes/sample_data/create_sample_data.py` - do we want that changed too?

Nope, we can't get rid of that since those (faulty) data cannot be produced with iris. However, this should not matter since this script is not used during the test. I only used it to create the sample data and I thought it would be good to keep it in the repo +1

excellent! Let's wait until tomorrow morning see if any of the other folks want to have a look, if no answer, fire it to master, bud! 🍺

[stefsmeets]

Hi @schlunma , nice work, looks much cleaner!

1. The definition of the SAMPLE_PATH variable is repeated at the top of every file. It would be nice if this could be single-sourced. Also, would it be possible for you to change the definition from using os.path to pathlib.Path while at it (I already made a few starts)? This will be much easier to work with and maintain.

2. Could you put the scripts to generate the .nc files somewhere (maybe directly in the sample_data directory) so that the data can be regenerated or updated if needed?

3. Finally, we already have a set of tests with sample data here: https://github.com/ESMValGroup/ESMValCore/tree/master/tests/sample_data/ . Could we call your directory test_data to avoid a name clash?

[valeriupredoi]

@stefsmeets very good idea about adding instructions how to create those standard test files, cheers! About Pathlib - you know me, am impartial 😁
More generally speaking, I need to put together a small reproducible test that replicates the issue related to data realization when run on multiple threads and the SegFault we get so we can ship it to the iris folk - will do this now 👍

[valeriupredoi]

let's get this one done and merged, I'd like to continue the fixing trials by reducing the size of tests, am fairly sure it's the size of the tests that's causing the SegFaults as I say in this comment - Manu, if you don't have time I can tend to Stef's comments 👍

[schlunma]

I already addressed them and will push them after the meeting 👍

[valeriupredoi]

I already addressed them and will push them after the meeting +1

total boss! 🍺

[schlunma]

Thanks for the review @stefsmeets !

1. The definition of the `SAMPLE_PATH` variable is repeated at the top of every file. It would be nice if this could be single-sourced. Also, would it be possible for you to change the definition from using `os.path` to `pathlib.Path` while at it (I already made a few starts)? This will be much easier to work with and maintain.

I changed os.path to pathlib and moved the sample path to a fixture in conftest.py. I'm quite happy with pathlib, although it's a pity that iris can't handle Path objects at the moment.

2. Could you put the scripts to generate the `.nc` files somewhere (maybe directly in the `sample_data` directory) so that the data can be regenerated or updated if needed?

The script is already there: tests/integration/cmor/_fixes/test_data/create_test_data.py

3. Finally, we already have a set of tests with sample data here: https://github.com/ESMValGroup/ESMValCore/tree/master/tests/sample_data/ . Could we call your directory `test_data` to avoid a name clash?

Renamed to test_data.

[valeriupredoi]

good job @schlunma - this change reduces the statistical rate of SegFaults from 16% to 5/80=6% (16% from this comment)

[stefsmeets]

Cheers @schlunma , I must have missed the script to generate the data on my first pass (I think the diff was probably too big for it to show 😅). Nice work, hope this helps reduce the number of seg faults!

[valeriupredoi]

cheers @schlunma and @stefsmeets 🍺