Re-organize coreg.py

[adehecq]

Summary

This PR is a (big) step towards making the use of coreg simpler and more consistent, as stated in #327.
NB: Some previous commits were accidentally added to this PR, to see the actual changes, use this link.

Here is a summary of the main changes:

• All coreg apply and fit functions accept either an xdem.DEM instance, or a numpy array, transform and CRS, i.e. the minimum information required to convert the output to a Raster/DEM (only array and transform were needed before).
• Replace all Coreg core functionalities with either standalone functions or xdem/geoutils functions:
  • Replace calculate_slope_and_aspect with xdem.terrain attributes -> actually reverted to old implementation, that is much faster. I left the xdem option commented, and could be reverted once xdem terrain attributes are sped-up, but in any case, it seems sufficient and more computationally efficient, to use a simple np.gradient rather than more complex gradient. Renamed the function to _calculate_slope_and_aspect_nuthkaab to avoid misuse as discussed in xdem.coreg.calculate_slope_and_aspect should (soon) be deprecated. #150.
  • Replace subsampling block with geoutils’ subsampling
  • Add a standalone function to create inlier_mask (load vectors, remove low/high slopes, outliers…)
  • Add function to calculate standard stats before/after coregistration. Defaults are "count", "mean", "median", "nmad", "std" but can be set by the user.
• Add an option resample to the apply methods to avoid resampling the data (and hence spreading NaNs). This is currently implemented for BiasCorr, Deramp, ZScaleCorr and NuthKaab. Resampling is necessary for all non-rigid transformations. In the case of NuthKaab, the output DEM has a different transform (and hence must be reprojected before calculating a ddem). This is useful if other processing follow the coregistration such as a second coreg method or regridding.
• Update dem_coregistration function to use apply's option resample=False and take as input xdem.DEM instances on top of filenames.
• remove the hmode/vmode in dem_coregistration since it's not used at the moment. The idea was possibly to allow two different inlier_masks for the horizontal vs vertical alignment.
• Remove unused/redundant functions (filter_by_range, filtered_slope, apply_xy_shift, apply_z_shift, dilate_mask option, duplicates of deramping)
• Add associated tests (in particular for resample option, create_inlier_mask and dem_coregistration).

To be addressed later

• Call calculate_ddem_stats in coreg.apply to get standard statistics -> This is more complex than expected, because reference DEM and inlier_mask are unknown in apply.
• Add plotting step in all apply methods, and in some fit methods, like N&K. -> same issue as previous
• Update Coreg so that coregistration params are more easily output than currently with _meta
• Modify fit to return coregistered DEM, so that apply is optional (to avoid re-doing calculation) -> turns out it only matters for CoregPipeline and NuthKaab (in the latter case, the shift is calculated in one go with apply instead of sequentially, so it may be beneficial).
• Update Coreg.error to not require re-running apply another time. -> will be addressed later
• Move function mask_as_array?
• Update documentation
• Make the resampling method (bilinear, cubic) an argument along the new resample argument
• Make the statistic for vertical shift (currently hardcoded np.nanmedian in N&K a parameter

[adehecq]

Thanks for the small fixes @rhugonnet !
Now there's a weird thing, you need to approve again, as I can't approve the PR myself 😆
We also need to fix the test error.

[erikmannerfelt]

A lot of great work, Amaury! I have some comments but generally I agree with all decisions made.

As you may notice, I've been out of the game for a while so some comments/questions may be self-explanatory to you and Romain at this point. I'm trying my best to get back!

[adehecq]

Some tests fail. I've checked all of them:

• test_coreg fails due to a bug in an older version of geoutils, but fix in the latest here: Fix small bug in spatial_tools geoutils#326
• test_docs fails for reasons that do not seem related to this PR. It fails on code/spatialstats_heterosc_slope.py, but the code itself runs without problem locally. But the exec statement in test_docs raises a weird error: "*** RuntimeWarning: numpy.ndarray size changed, may indicate binary incompatibility. Expected 16 from C header, got 96 from PyObject" When trying to run the commands manually, it even fails in an earlier script...
• I haven't looked in detail at the errors in spatialstats.py, but again, I doubt they are related to this PR, but maybe to recent changes in geoutils?
  Your input would be useful on this @rhugonnet !

[erikmannerfelt]

Some tests fail. I've checked all of them:

* test_coreg fails due to a bug in an older version of geoutils, but fix in the latest here: [Fix small bug in spatial_tools geoutils#326](https://github.com/GlacioHack/geoutils/pull/326)

* test_docs fails for reasons that do not seem related to this PR. It fails on `code/spatialstats_heterosc_slope.py`, but the code itself runs without problem locally. But the exec statement in test_docs raises a weird error: "*** RuntimeWarning: numpy.ndarray size changed, may indicate binary incompatibility. Expected 16 from C header, got 96 from PyObject" When trying to run the commands manually, it even fails in an earlier script...

* I haven't looked in detail at the errors in spatialstats.py, but again, I doubt they are related to this PR, but maybe to recent changes in geoutils?
  Your input would be useful on this @rhugonnet !

@adehecq, do you want to merge anyway and add an issue ASAP, since this seems unrelated to your PR? The only consequence is an ugly "Faling" badge on the front page.

[rhugonnet]

For the errors in test_spatialstats, they might actually be related to the change in nodata of the ddem?

We also had to change this test in this PR for it to pass (2316 changed in 0)

    @pytest.mark.parametrize("rst_and_truenodata", [(ref_dem, 0), (tba_dem, 0), (ddem, 2316)])  # type: ignore
    # Note: Following PR #329, no gaps on DEM edges after coregistration

One way to be sure: you could run your tests locally with/without the PR (main and this branch) to check if it fails?

And I agree for leaving aside the weird error in test_docs and opening an issue! 😄

[rhugonnet]

Just checked this: with the same environment, all tests passing on the main branch. The change for values in test_spatialstats are definitely linked to the new data that is replacing NaNs in the ddem.
Updating tests...

[rhugonnet]

Actually, all the tests that were failing except the one linked to GlacioHack/geoutils#326 were due to the changes in this PR. I fixed them all.
We can merge!

[rhugonnet]

Now I'll open a PR from a new branch pulling the latest GeoUtils, to see if releasing a new GeoUtils version will fix the issue and if it will create others.