Fix cmip6 models

[npgillett]

Before you start, please read CONTRIBUTING.md.

Tasks

• Create an issue to discuss what you are going to do, if you haven't done so already (and add the link at the bottom)
• This pull request has a descriptive title that can be used in a changelog
• Add unit tests
• Circle/CI tests pass. Status can be seen below your pull request. If the tests are failing, click the link to find out why.
• Codacy code quality checks pass. Status can be seen below your pull request. If there is an error, click the link to find out why. If you suspect Codacy may be wrong, please ask by commenting.

If you need help with any of the tasks above, please do not hesitate to ask by commenting in the issue or pull request.

---

Closes #628

[bouweandela]

@npgillett Thank you for sharing these fixes! Could you please have a look at the code quality suggestions by Codacy? You can read more about contributing Python code in our contribution guidelines.

If you do not know where to start with adding unit tests, maybe @valeriupredoi would be able to help out?

[valeriupredoi]

I have just pushed a test for GFDL ESM4 siconc fix, that should give you a good enough example on how to write the tests for the rest of the fixes, basically:

• follow existing examples;
• create a data file that should mimic the actual data with its issues;
• fix that test data file and...
• check for the existence of the fixes and their parameters.

You can write a draft test and run it immediately via pytest, checks for fails, correct them etc...and then when you think you done you should check the coverage from test-reports - the coverage for your fix is nicely highlighted in a local webpage eg file:///home/USER/ESMValCore/test-reports/coverage_html/esmvalcore_cmor__fixes_cmip6_gfdl_esm4_py.html and there you seee which statements are not covered and need to be tested;

Finally please run isort on the test file to sort the imports and run prospector see if you have any style issues (can correct most of them running yapf -i [test_file] 🍺

[valeriupredoi]

Note that for your siconc fix I reckon there's a problem how you build the aux coord:

        typesi = iris.coords.AuxCoord(
            'siconc',
            standard_name='area_type',
            long_name='Sea Ice area type',
            var_name='type',
            units='1',
            bounds=None)

since you are assigning the coordinate data (points) as siconc?

[npgillett]

Thanks @valeriupredoi and @bouweandela for the help so far! I managed to revise the code to pass the Codacy checks by adding and removing spaces etc. According to the list my code failed tests in Docker Cloud. But when I click on details I get a 404 - page not found message. What do I need to change to pass the Docker Cloud test? Thanks!

[npgillett]

Thanks @valeriupredoi for creating the test for the GFDL-ESM4 siconc fix. I took a look at this, but looks like it might take me a while to write something similar for each other model fix...

[npgillett]

@valeriupredoi Thanks for your comment about the typesi fix above. I have to admit that I don't fully understand the fix, but I found it in an earlier version of ESMValTool - see e.g. here . The code does run without giving an error when the fix is included, but beyond this I can't verify that it is correct. How would you suggest changing this?

[bouweandela]

According to the list my code failed tests in Docker Cloud. But when I click on details I get a 404 - page not found message. What do I need to change to pass the Docker Cloud test?

Sorry about that, we're still in the process of setting that up, so you can ignore those tests for now. The tests that matter are ci/circleci and Codacy.

[bouweandela]

I managed to revise the code to pass the Codacy checks by adding and removing spaces etc.

Note that you can do this automatically by using yapf, as recommended in our contribution guidelines.

[bouweandela]

I'm slightly surprised to see flake8 checks failing in code that you didn't change though, I'll have a look at that tomorrow.

[bouweandela]

I'm slightly surprised to see flake8 checks failing in code that you didn't change though, I'll have a look at that tomorrow.

This was solved after merging the latest master branch into this branch. Could you please have a look at the remaining issues? Let us know if you need help.

[valeriupredoi]

fixed a few things there, have a looksee @npgillett - could you pls make sure all fixes have tests attached to them? Lemme know if you need any assistance 🍺

[npgillett]

@valeriupredoi Thanks very much for your help on this so far - would you be able to review this merge request? I have lost track of this a bit, but it looks like only the Dockercloud test is failing now, and the details link points to a page not found, so I'm not sure if that's a real problem. Hopefully we can get this merged soon... Thanks very much!

[valeriupredoi]

sure thing Nathan @npgillett - will have a look first thing in the morning 🍺

[valeriupredoi]

OK fixed all the tests and added missing ones 🍺

[valeriupredoi]

@bouweandela you wanna have another last look before I merge? 🍺