-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Makefile initial installation #630
Makefile initial installation #630
Conversation
523f1b5
to
88ebde1
Compare
I am not very familiar with Makefile builds (@adehecq is much more). If my understanding is correct, this would replace/improve the current developer installation via For us (even before xDEM, and on Python 2), using the We could also coordinate with Richard Barnes to solve the issue. |
88ebde1
to
f6fc3b9
Compare
@rhugonnet the aim of this makefile is to install easily xdem in develoment mode with pip, so it would not replace/improve |
8cf6306
to
885e20b
Compare
885e20b
to
977358d
Compare
Hello !
|
@duboise-cnes Could you give us your opinion on the dependances issues? |
@rhugonnet Would you agree to update the readme by indicating that a pip install is viable? |
We also should update documentation : https://xdem.readthedocs.io/en/stable/how_to_install.html with the makefile option for contributors |
977358d
to
10df937
Compare
@adebardo For now I have commented the |
Yes, let's add the |
10df937
to
4a85f06
Compare
Makefile is not really "needed" absolutely but it is common tool in linux development and a simple wrapper for development practise. I understand that conda ease the way to have GDAL (even if you are not root on your pc). But in our experience, we have tried to remove GDAL direct dependency because of difficulty of maintenance and use it through rasterio directly in python3 or other way. So I would propose two steps:
As for richdem, if not so maintained for different python, can the dependency be removed, included, updated as discussed (if i remember well) ? Again, maybe in another PR, not sure I have several comments on the code line by line in following comments. |
Thanks @duboise-cnes, it's much clearer! For GDAL/RichDEM: They are not dependencies for the users, but they are for developers (only used in tests). They are quite important as they verify all types of terrain attributes (slope, aspect, TPI, TRI, etc), custom reprojection in the same CRS (to avoid sub-pixel errors of Rasterio that haven't been fixed in more than a year), etc. Same for RichDEM to check curvatures. I'd advocate for keeping them. But if we want to remove them, that's doable too 🙂. |
8dbb2b9
to
f9f0bf9
Compare
It was clear for me that it was only for developers. But even for developers, the dependency can be complex. Richdem seems not so well maintained and with the first move on the licence, I would advocate also to not have the development installation blocked by it. Same approach can be taken. |
f9f0bf9
to
f26b986
Compare
@adebardo @rhugonnet @duboise-cnes The PR has been updated, I have found a way to solve the GDAL configuration problem. Now the makefile works if the user has GDAL on the system (if not the makefile gives instructions on how to install it). We still have constraints on richdem with python > 3.10, so for the moment the makefile is only configured for python3.10. |
@vschaffn Perfect for the solve! Not much to comment on my side, looks great :) |
f26b986
to
8235de2
Compare
@vschaffn @adebardo @duboise-cnes I realized today that RichDEM is simply not distributed on PyPI since 6 years: https://pypi.org/project/richdem/. It's only regularly updated on conda: https://anaconda.org/conda-forge/richdem. So the only option to make it work in the Makefile would likely be to build it from source, as recent releases are available on GitHub: https://github.com/r-barnes/richdem/releases. Some discuss this here: r-barnes/richdem#65. Otherwise the removal we talked about to |
Resolves #582.
Description
This PR introduces the initial Makefile for the
xdem
project, aimed at streamlining the installation process for developers on Linux systems. By implementing a Makefile, we provide a simple command-line interface for common setup tasks, enhancing the developer experience and reducing the potential for errors during installation.Key Changes:
Makefile Structure: The Makefile has been created based on the existing structure of the
demcompare
project.Targets Implemented:
help
: Displays help information about available targets.venv
: Creates a virtual environment in the specified directory if it does not exist.install-gdal
: Installs the version of GDAL corresponding to that of the system.install
: Installs thexdem
package in editable mode along with necessary dependencies and sets up pre-commit hooks for Git.clean
: A comprehensive clean target to remove build artifacts, virtual environment, and pre-commit hooks.test
: Run pytest (if GDAL is in the virtual environment).Update dependencies:
The dependencies used in each sections (required, optionnal, test, doc) have been updated to match with the projet.
Documentation
The documentation https://xdem.readthedocs.io/en/stable/how_to_install.html, README.md and CONTRIBUTING.md have been updated with the
make install
command for contributors.Constraints
richdem
is not stable enough to build a wheel on python > 3.10. For the moment, themake install
command only works on python 3.10 and fails if this version is not installed on the system.