Always prepare data in accuracy worflow (coreg/no coreg process)

[marinebcht]

Resolves #840

Currently, in the CLI, the data are reprojected and intersected (=preparation) only when the user enables coregistration. The purpose of this PR is to always prepare the data, even when the coreg process is not asked.

Thus, the _prepare_datas_for_coreg() function is now called _prepare_datas() at the beginning of the process, if the value of sampling_grid is not None, outside of the if self.compute_coreg condition.

Config

Since the accuracy workflow requires the sampling_grid parameter to perform the reprojection, coregistration or not, this parameters was moved to inputs :

inputs:
  reference_elev:
    path_to_elev: "xdem/example_data/Longyearbyen/data/DEM_2009_ref.tif"
  to_be_aligned_elev:
    path_to_elev: "xdem/example_data/Longyearbyen/data/DEM_1990.tif"
  sampling_grid: "reference_elev"

It can be equals to :

• "reference_elev" to reproject to_be_aligned_elev to reference_elev
• "to_be_aligned_elev" to reproject reference_elev to to_be_aligned_elev
• null, if no reprojection/intersection wanted BUT its is required in case of a coreg process (Error: 'In case of a coregistration process, "sampling grid" must be set to ''"reference_elev" or "to_be_aligned_elev"') AND the workflow may generate an error if the two inputs have not the same shape, transform and CRS.

As the process key became more critic, I added it in the template_config (=True) here

/!\ During YAML reading, None is serialized as "None". A function was added to correct this behaviour + little note in the doc.

Test

The test consists of :

• cropping the input image ref_dem or tba_dem (number of pixels = N2)
• samling_grid = the dem choosen to be cropped
  We obtaining Total counts = N2 in both stats. If samling_grid = null, it raises of an error.

The two intermediate rasters to_be_aligned_elev_reprojected.tif and reference_elev_reprojected.tif are now generated by _prepare_datas() when the level is greater than 1.

Also, I added a test for the _prepare_datas() function: test_prepa_data, covering cropped inputs, no intersection and both sampling_grid values.

CLI improvment

For topo and accuracy workflow, I proposed to add also the optional --output (already present for --config) for --template-config to save the dict directely in a file (print if not).

xdem topo --template-config template_config.yaml gives:

[rhugonnet]

@marinebcht I know this is a draft, but for info, the line changes of the PR seem to be mixed with recent history, so we can't see what the new changes are.

A good solution to tackle this is usually to do beforehand:

git checkout main  # On your fork
git fetch upstream  # Upstream being the GlacioHack repo
git rebase upstream/main  # Rebase so that history starts at your PR and not before
git checkout -b my_new_branch  # Start fresh on new branch

To correct a posteriori, I usually have to Google to know what to do... 😅

[marinebcht]

I repaired my branch :) Had to git rebase + git skip so many time .. ^^
@rhugonnet Thanks you for the tips, saved this in a script for next time (this is not my first time!)

[belletva]

I repaired my branch :) Had to git rebase + git skip so many time .. ^^ @rhugonnet Thanks you for the tips, saved this in a script for next time (this is not my first time!)

That's perfect for the issue for the new arrivals document ;)

[marinebcht]

@belletva @adebardo @rhugonnet @adehecq I think this PR is done :)

[rhugonnet]

Great, thanks @marinebcht! The PR was very well explained, and reviewing the code changes I don't have much to add 🙂

One question: I see you null everywhere. From the new note in the doc, it sounds like the reason behind it is that None is not accepted by YAML?
If yes, should we remove None entirely from the table of arguments? (and only leave the note that explains that "null in YAML = None in Python"; I think the text of the note can be clarified further by explaining in this direction, given that CLI users will only know about null)

[marinebcht]

Thanks a lot @rhugonnet 😎

[marinebcht]

@rhugonnet @belletva @belletva
I added the function was added to correct None-to"None" behaviour + little note in the doc + test

If it is ok, we can merge :)

[rhugonnet]

@marinebcht Perfect!

I think we can slightly adjust the note: The value null in the YAML file, representing the absence of a value or a null value, is serialized as None in the dictionary.

CLI users don't know what "the dictionary" is, I think? It's internal mechanics, just for us to convert the YAML into Python API arguments. They don't have to know about it. So we can remove!

Maybe we adjust into something like: We accept both null and None values interchangeably in YAML files, which correspond to None in the Python API.
What do you think?