GlacioHack / xdem

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

`resolution` attribute missing in `terrain.aspect()` #490

Closed geograFie closed 3 months ago

geograFie commented 3 months ago

What is the resolution attribute in e.g. terrain.slope() needed for? From what I understood, it refers to the pixel cell size (e.g., 10 meters).

However, resolution is missing in terrain.aspect() and defaults to 1.0. Why is it not possible / necessary to specify this for the aspect as well, since this is calculated based on the slope?

rhugonnet commented 3 months ago

Indeed this is unclear in the docstring, the resolution argument is optional and only there in case you want to pass an array (and not a DEM/Raster object).

I clarified this behaviour in the documentation in a recent PR not yet released, which also allows to call terrain functions directly from the DEM dem.slope(): https://github.com/GlacioHack/xdem/pull/478 (to avoid needing having the choice of that argument in the first place, it is removed from the new docstring)

I'll also clarify the old docstrings in terrain.py!

rhugonnet commented 3 months ago

And for your point on having resolution in aspect(): This is normal, the orientation of the slope does not depend on the resolution of the grid (except if it is not an equal grid dX=dY, but this is not supported anyway). Only the steepness of the slope changes with different grid sizes, hence the argument existing in slope().