Added land/sea fraction weighting preprocessor

[schlunma]

This PR adds a land/sea fraction weighting preprocessor as suggested by @ChrisJones-MOHC in #298 (I borrowed some text of your issue for the documentation, I hope that's okay 😁). With this change, many derived variables are not needed anymore, so there are a few changes necessary in ESMValTool after that.

Requires #230.

• 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
• Public functions should have a numpy-style docstring so they appear properly in the API documentation. For all other functions a one line docstring is sufficient.
• If writing a new/modified preprocessor function, please update the documentation
• 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.
• Please use yamllint to check that your YAML files do not contain mistakes
• If you make backward incompatible changes to the recipe format, make a new pull request in the ESMValTool repository and add the link below

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 #298. Necessary for ESMValGroup/ESMValTool#473.

Necessary changes for ESMValTool: ESMValGroup/ESMValTool#1466.

[schlunma]

In my opinion explicitly having to set an option to False is not silently ignoring it (note that there is also a warning). I think it's comparable to the --skip-nonexistent switch, which also allows missing data if explicitly desired.

Is there a reason to want to use a single preprocessor profile for all datasets? E.g. because you want to apply multi model mean or masking functions?

Because it's way more user friendly. Using two different preprocessors requires another block in the variables section for the same variable with its own additional_datasets, etc.

BTW, all the _grid variables (which implicitly used this preprocessor) had the same behavior (and there was no even a warning).

[schlunma]

It would be really nice if we could merge this soon, since I need it for ESMValGroup/ESMValTool#473.

Maybe someone else has an opinion on this? @valeriupredoi @mattiarighi

[valeriupredoi]

Good by me, man, travelling but will approve it in an hour or so when I get back to Reading 🍺 Dr Valeriu Predoi. Computational scientist NCAS-CMS University of Reading Department of Meteorology Reading RG6 6BB United Kingdom

…

On Tue, 12 Nov 2019, 13:45 Manuel Schlund, ***@***.***> wrote: It would be really nice if we could merge this soon, since I need it for ESMValGroup/ESMValTool#473 <ESMValGroup/ESMValTool#473>. Maybe someone else has an opinion on this? @valeriupredoi <https://github.com/valeriupredoi> @mattiarighi <https://github.com/mattiarighi> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#331?email_source=notifications&email_token=AG5EFI5HK2HUO6M5ENCCEZ3QTKXQBA5CNFSM4JEDTMW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOED2JGJA#issuecomment-552899364>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AG5EFI5ZOLVTTUYZGRFTQWTQTKXQBANCNFSM4JEDTMWQ> .

[mattiarighi]

I'm fine with this, although I had no time to test it.

[schlunma]

@bouweandela, do you have further comments?

[bouweandela]

I'm still not convinced that this is a good approach, for the reasons mentioned above. If you really insist on using one preprocessor profile to do two different preprocessing chains, maybe an exclude argument taking a list with the names of datasets, like we have for the multi model functions, could be an option instead of strict? At least that makes it immediately clear from the recipe which datasets will skip this preprocessing step.

[schlunma]

All right, that sounds like a reasonable compromise to me. I will implement this as soon as possible.

[schlunma]

@bouweandela I made all necessary changes, hopefully we can merge this soon now.

[bouweandela]

Looks good to me now. Could you please also make the pull request to adapt exiting recipes so they use the new preprocessor function instead of a derived variable?

[bouweandela]

@mattiarighi Please test before merging, I only looked at the code.

[schlunma]

Will do 👍

[mattiarighi]

I can only test this together with the corresponding PR in ESMValTool where the recipes are adjusted.

[schlunma]

You can test this recipe in this branch.

[mattiarighi]

Tested successfully! 🎉

[bouweandela]

@schlunma @mattiarighi Is there a corresponding pull request in the ESMValTool repository already? Or is this not needed? Already done?

[schlunma]

@bouweandela Only small changes were necessary, they have already been addressed in ESMValGroup/ESMValTool#1466.