search

TopoToolbox / pytopotoolbox

Python interface to TopoToolbox
https://topotoolbox.github.io/pytopotoolbox/
GNU General Public License v3.0
1 stars 2 forks source link

GridObject, fillsinks and plotting added #8

Closed Teschl closed 3 months ago

Teschl commented 3 months ago

I added the GridObject, fillsinks(), imagesc(), and imageschs() to the package. In the tests folder, there is a Jupyter notebook file showcasing a bit of the functionality. Later on, I will add a smaller example matrix so that the changes from running fillsinks are more visible.

At this point, I just pasted the libtopotoolbox.so into the topotoolboxdirectory. If you want, I can remove it so that everyone can build it themselves.

wkearn commented 3 months ago

Excellent work, @Teschl! I'll go through this more thoroughly tomorrow, but here are some initial thoughts:

We probably don't want to commit libtopotoolbox.so to the repository. I'm still not completely sure what the best way to deliver the shared library to users is. I think we probably need to configure the Python build system to compile the library and put it in a standard location, and then we can deliver the whole Python package with the compiled library as a wheel or something. I've been reading through the scikit-build-core docs, which seems like one way to do it, but I haven't determined if that is the best solution for our needs.

I am also wary of committing GeoTIFFs to the repository. I know we want some real DEMs around for testing and documentation, but it is generally good practice not to commit large binary files. Perhaps we can provide a way for the data to be downloaded from somewhere when the package is installed. In Julia I usually use DataDeps.jl for this kind of thing. Maybe there is something similar for Python. At the very least, we could download examples as part of a setup script. See #9 for more discussion.

Teschl commented 3 months ago

I definitely agree; it makes sense not to have GeoTIFFs just lying around in the repository. I think it would probably be better to also move the Jupyter notebook example files from tests/ to a directory like examples/ to reserve tests/ solely for actual tests. I'll look into having the binary files be downloaded automatically. We can probably just work with this repository: wschwanghart/DEMs.

Regarding the automatic build, we could probably use shell/batch scripts. That would only require the user to download both repositories and have them in the same folder (If they want to build it themselves).