Refactor preprocessor settings related to fx files

[zklaus]

Hi @valeriupredoi, this is more the kind of refactoring I had in mind. I think it is clearer and more easily extensible for future preprocessors. It's left to also unify _update_fx_settings_mask and _update_fx_settings_stats, which should be possible if one untangles the 6 levels of indentation in the stats method, but at the moment I am out of time.

[valeriupredoi]

very nice, dude! I reckon this is disentangled enough to be able to plug in the changes that we'll have to make for the iris-way of handling the fx variables for preproc too. DO you want to disentangle _update_fx_settings_stats or should I do it? 🍺

[zklaus]

I would be very grateful if you could do it.

[valeriupredoi]

on it, mate 🍺

[valeriupredoi]

what say you @zklaus ?

[valeriupredoi]

there seems to be an issue with github, I merged the parent and fixed the conflict but it's not showing - and now I cant even comment :grrr:

[valeriupredoi]

ahh it's back now

[valeriupredoi]

to merge or not to merge, dis iz the question :hamlet: - what do you reckon @zklaus 🍺

[zklaus]

I think it's a good step forward, so merge.

Next step: unify _update_fx_settings_mask and _update_fx_settings_stats.

For this it might be helpful to note that _get_spatial_stats_fx_vars really has nothing about spatial or stats things, but rather just gets the requested fx vars from the setting.

Looking at line 534 we notice that when the update method is called, the settings parameter already contains the pertinent subsection so that the manual distinction in lines 520, 524, and 528 is unnecessary.

Indeed, this automatic determination of fx variables makes one wonder if that could replace the manual listing of sft{lf,of,gif} entirely.

[valeriupredoi]

but _get_spatial_stats_fx_vars needs to be called because it's the only function that gets the fx_files attr from the step (the others - mask landsea and weighted - don't need to). Kinda hard to unify the two funcs coz one need the call to get_output_file - ideas? 🍺

[zklaus]

Well, in one we place the fx file as determined by _get_correct_fx_file in the fx_dict, in the other the result of get_output_file. What's the reason for this difference?

[valeriupredoi]

get_output_file is used as a patch to get the already preprocessed and written to disk fx file, since the preprocessing and file writing is done independently of the whole fx workflow and is done like for any other variable

[zklaus]

But why is that needed for some files, but not for others? What if a custom order is applied and both mask and area files have been regridded, for instance?

[bouweandela]

@zklaus and @valeriupredoi I renamed the pull request that hopefully describes what happens here. Note that I use those pull request titles in the change log, so it would be great if you could keep an eye on that they make sense to other people too.

[valeriupredoi]

yeah good call man - can you not rename the parent just yet pls? I will rename it to something similar but only after merge (if it'll ever happen 😁 ) but I need the parent-child denominator so I know what's left to be done; and I will close #439 after too