GlacioHack / xdem

Analysis of digital elevation models (DEMs)
https://xdem.readthedocs.io/
MIT License
123 stars 37 forks source link

Fuse `Coreg` point functions and add consistent Raster-Point logic #480

Closed rhugonnet closed 3 months ago

rhugonnet commented 4 months ago

Summary

This PR adds point-raster and point-point support in the base Coreg class, with modifications to properly inherit it through CoregPipeline and BlockwiseCoreg, making the treatment of point-raster coregistration fully consistent for the entire coregistration module. It adds point-raster support for all BiasCorr classes, and maintains current point-raster support for AffineCoreg classes.

In practice

Any gpd.GeoDataFrame can now be passed with a z_name argument defining the column name representing the elevation point cloud data, and it can be passed either as reference_elev, or to_be_aligned_elev, or both. In the future, it will be easy to pass an xdem.ElevationPointCloud() that knows the z_name to avoid multiplying arguments, see #463.

A gpd.GeoDataFrame is always compared to a gu.Raster using gu.Raster.interp_points, fully consistent and tested since https://github.com/GlacioHack/geoutils/pull/484. It mirrors Xarray.interp() behaviour, but should be faster on equal grids thanks to using directly map_coordinates. A gu.Raster is converted into points using to_points() (for tests or for fallback mechanism in the Coreg class discussed in #402). This still requires some fixes in GeoUtils to avoid passing too many arguments: https://github.com/GlacioHack/geoutils/issues/499.

Everything is done through fit() and apply().

Changes

This PR:

Additionally, this PR makes all UserWarning fail tests at the package level, and removes the individual warnings.simplefilter("error"). All other warnings that depend on our routines are either fixed, filtered or caught if expected during the tests.

Resolves #472 Resolves #479 Resolves #402 Resolves #475 Resolves #134 Resolves #404 Resolves #489 Resolves #376 Resolves #377

TO-DO:

For later

Reworking tests in test_affine and adapting some of the functions: more efficient if done in another PR after working on the AffineCoreg restructuration for being more modular. Opened #485.

rhugonnet commented 4 months ago

Tests all passing locally! :partying_face: To fix them in CI: probably need to release a 0.1.1 for GeoUtils that integrates the recent interp_points fixes!

rhugonnet commented 4 months ago

@erikmannerfelt Long time since we had that revamping discussion, it's finally done!! :grin:

rhugonnet commented 4 months ago

Tests pass locally but require https://github.com/GlacioHack/geoutils/pull/501

rhugonnet commented 3 months ago

Alelujah OpenCV finally installs on CI again... I've opened a Wiki page to remember how to solve this: https://github.com/GlacioHack/xdem/wiki/Installing-OpenCV-in-CI.

adehecq commented 2 months ago

Just thinking: should we add a point cloud to our sample data to document these new features in the documentation?

rhugonnet commented 2 months ago

Yes, maybe once we have #463!