Proposal to deprecate the callback argument from esmvalcore.preprocessor.load in v2.8 and remove it in v2.10.

[bouweandela]

I would like to deprecate the callback argument from the esmvalcore.preprocessor.load function because it

1. does not seem likely that anyone uses this as it cannot be meaningfully set from the recipe
2. prevents implementation of a simple Dataset.load function in Support wildcards in the recipe and improve support for ancillary variables and dataset versioning #1609. See the example notebook for the intended use of Dataset.load. This function would do everything related to loading the data: fixes, cmor checks, concatenation, and attaching ancillary variables.

@ESMValGroup/esmvaltool-coreteam Would this change cause problems for your work?

[schlunma]

Sounds good to me!

[valeriupredoi]

can we maybe convert it to a different load engine eg an xarray dataset or a GRIB file? I am starting to think how to support other formats, like GRIB

[zklaus]

can we maybe convert it to a different load engine eg an xarray dataset or a GRIB file? I am starting to think how to support other formats, like GRIB

Iris supports Grib perfectly fine; no change needed.

I am ok with removing the callback as-is. Mid- to long-term, we might want to use the (extended?) Iris callback functionality for some of the fixing. That should help avoid most fix_file fixes.

[valeriupredoi]

@bouweandela I started working on this here #1901

[bouweandela]

Thanks V! I did the same thing in #1801, but then I realized that this change was backward incompatible, even if nobody might be using the feature, so I thought it might be better to deprecate it instead and add some workarounds so it keeps working even with the new esmvalcore.dataset.Dataset.load function.

[valeriupredoi]

Thanks V! I did the same thing in #1801, but then I realized that this change was backward incompatible, even if nobody might be using the feature, so I thought it might be better to deprecate it instead and add some workarounds so it keeps working even with the new esmvalcore.dataset.Dataset.load function.

Here's a question for you - as it stands now I removed all instances of callback=callback in the load function (including in the actual function definition) but given that this callback was indeed called from a few places (like regrid or ancil_vars), and in each case with the concatenate_callback() func I feel like we should add that functionality back into each of these preprocessors so we don't run into metadata-related issues; do you think we should do that? Or add a check inside load() for the stuff that the callback was doing?

[valeriupredoi]

we could just remove it and not deprecate it since only two cmorizers use the load callback, apart from that no recipe afaik - we can just fix those cmorizers?

[valeriupredoi]

I did the same thing in #1801

Sorry for starting a new PR, bud - I completely missed the linked PR upstream - my old Firefox has lost the ability to remove floating temporary popping windows so I might have missed that hidden behind one of those (need to upgrade, yes, I know 😁 )

[remi-kazeroni]

Hi @valeriupredoi and @bouweandela, shall we move this to v2.9 since the corresponding PR was shifted to v2.9? Or do you think you would have time to address this issue during this week? Thanks 👍

[valeriupredoi]

@remi-kazeroni I think @bouweandela is rather under a lot of snow with #1609 and the other baby PRs belonging to it - maybe we can push this to 2.9 since it's not a history-defining change anyways?

[bouweandela]

In #1609, I deprecated the feature and set the version for removing it to v2.10. I'll bump this issue to v2.10 too then.