Skip to content
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

Remove GDAL and RichDEM dependancy from tests #675

Merged
merged 7 commits into from
Jan 23, 2025

Conversation

vschaffn
Copy link
Contributor

@vschaffn vschaffn commented Jan 9, 2025

Resolves #671.

Description

The purpose of this PR is to eliminate dependencies on GDAL and RichDEM in the xDEM tests, thereby simplifying the installation process for developers. The scripts related to GDAL and RichDEM have been relocated to the xdem-data repository, where they generate and save the ground truth data in .tif format. These .tif files are then imported into xDEM during test execution.

Changes

  • The functions related to GDAL and RichDEM have been removed from conftest.py and the test files, and transferred to xdem-data. These functions have been replaced by calls to the ground truths generated within xdem-data.

  • New functions have been added to conftest.py and functions in examples.py has been modified to retrieve the ground truth data from the xdem-data repository.

  • The GDAL and RichDEM packages have been removed from the configuration files.

  • The Makefile has been simplified and restored for Python versions above 3.10.

  • The version of the scipy package has been fixed to <1.15.0, as the new version causes issues with test_biascorr.py. A new issue has been opened to modify this when an update of scipy will be released Fix scipy<1.15.0 pin in virtual environment configuration files once issue in SciPy is resolved #677.

Note

Before merging:

Copy link
Member

@duboise-cnes duboise-cnes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review afterwards

tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
dev-environment.yml Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
@duboise-cnes
Copy link
Member

@adehecq @rhugonnet I have made some review, please tell if other remarks to be able to merge. The richdem, gdal removal was discussed with @rhugonnet .

Have a good day

@rhugonnet
Copy link
Member

Impeccable 👌, I have nothing to comment on. Great work!

I'll have a quick look at the SciPy issue.

@vschaffn vschaffn force-pushed the 671-remove_richdem_gdal branch from 951e2a9 to 3686704 Compare January 22, 2025 09:42
@vschaffn vschaffn force-pushed the 671-remove_richdem_gdal branch from ca232a2 to 9171742 Compare January 22, 2025 10:29
@adebardo
Copy link

@duboise-cnes one of your comment does not allow the merging, i don't understand how it works on github x)

@rhugonnet
Copy link
Member

Ah yes 😅. I think it's because when the a reviewer selects "Request changes" when posting the review (as did @duboise-cnes here), they need to re-review with approval later on!

@duboise-cnes
Copy link
Member

Ah yes 😅. I think it's because when the a reviewer selects "Request changes" when posting the review (as did @duboise-cnes here), they need to re-review with approval later on!

Sorry, I am not that familiar with github review process. I normally have rectified my mistake now, you can normally merge

@adebardo adebardo merged commit 5c5639d into GlacioHack:main Jan 23, 2025
19 checks passed
@vschaffn vschaffn deleted the 671-remove_richdem_gdal branch January 23, 2025 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[POC] Remove richdem/gdal from tests
4 participants