Remove GDAL and RichDEM dependancy from tests #675
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,7 +61,6 @@ test = | |
| flake8 | ||
| pylint | ||
| scikit-learn | ||
| doc = | ||
| sphinx | ||
| sphinx-book-theme | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,142 +1,76 @@ | ||
| import os | ||
| import tarfile | ||
| import tempfile | ||
| import urllib | ||
| from distutils.dir_util import copy_tree | ||
| from typing import Callable | ||
|
|
||
| import pytest | ||
|
|
||
| _TESTDATA_DIRECTORY = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "tests", "test_data")) | ||
|
|
||
| # Define a URL to the xdem-data repository's test data | ||
| _TESTDATA_REPO_URL = "https://github.com/vschaffn/xdem-data/tarball/2-richdem_gdal" | ||
| _COMMIT_HASH = "31a7159c982cec4b352f0de82bd4e0be61db3afe" | ||
|
There was a problem hiding this comment. I quickly checked if downloading was the best solution, and it seems fine and easy to maintain. Indeed, it might be better to download a tarball via a release. Is that what you were thinking? Also, we obviously need to update this PR once the xdem-data PR is merged and a release is created, so we can have the correct tarball/zip with a fixed version API. It doesn't seem like you're handling a version here. Maybe it's better to use a release, something like: https://github.com/glaciohack/xdem-data/releases/download/v1.0.0/xdem-data.zip There was a problem hiding this comment. @duboise-cnes I used the same download system as for importing the example data (Longyearbyen DEMs) here to maintain the same consistency and ease of maintenance. The example data are downloading from the main There was a problem hiding this comment. ok I understand therefore the rationale, you know better the code of xdem, i didn't know that there was the example download code. I would say that using requests is better than urllib for maintenance but these would change the entire also for the examples. And the download function could not be factorized with the example data, some code replication here. Not needed in my opinion. |
||
|
|
||
|
|
||
| def download_test_data(overwrite: bool = False) -> None: | ||
| """ | ||
|
There was a problem hiding this comment. I think that you can use requests et zipfile (or tarball as used equivalent) if a release ? For example from a well known AI ;) not used directly just for inspiration does it help ? |
||
| Download the entire test_data directory from the xdem-data repository. | ||
|
|
||
| :param overwrite: If True, re-downloads the data even if it already exists. | ||
| """ | ||
| if not overwrite and os.path.exists(_TESTDATA_DIRECTORY) and os.listdir(_TESTDATA_DIRECTORY): | ||
| return # Test data already exists | ||
|
|
||
| # Clear the directory if overwrite is True | ||
| if overwrite and os.path.exists(_TESTDATA_DIRECTORY): | ||
| for root, dirs, files in os.walk(_TESTDATA_DIRECTORY, topdown=False): | ||
| for name in files: | ||
| os.remove(os.path.join(root, name)) | ||
| for name in dirs: | ||
| os.rmdir(os.path.join(root, name)) | ||
|
|
||
| # Create a temporary directory to download the tarball | ||
| temp_dir = tempfile.TemporaryDirectory() | ||
| tar_path = os.path.join(temp_dir.name, "test_data.tar.gz") | ||
|
|
||
| # Construct the URL with the commit hash | ||
| url = f"{_TESTDATA_REPO_URL}#commit={_COMMIT_HASH}" | ||
|
|
||
| response = urllib.request.urlopen(url) | ||
| if response.getcode() == 200: | ||
| with open(tar_path, "wb") as outfile: | ||
|
There was a problem hiding this comment. do not needed with requests ? |
||
| outfile.write(response.read()) | ||
| else: | ||
| raise ValueError(f"Failed to download test data: {response.status_code}") | ||
|
|
||
| # Extract the tarball | ||
| with tarfile.open(tar_path) as tar: | ||
| tar.extractall(temp_dir.name) | ||
|
There was a problem hiding this comment. check the format with a versioned release in xdem-data. if tar ok, otherwise zip equivalent. |
||
|
|
||
| # Copy the test_data directory to the target directory | ||
| extracted_dir = os.path.join( | ||
| temp_dir.name, | ||
| [dirname for dirname in os.listdir(temp_dir.name) if os.path.isdir(os.path.join(temp_dir.name, dirname))][0], | ||
| "test_data", | ||
| ) | ||
|
|
||
| copy_tree(extracted_dir, _TESTDATA_DIRECTORY) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") # type: ignore | ||
| def get_test_data_path() -> Callable[[str], str]: | ||
| def _get_test_data_path(filename: str, overwrite: bool = False) -> str: | ||
| """Get file from test_data""" | ||
| download_test_data(overwrite=overwrite) # Ensure the test data is downloaded | ||
| file_path = os.path.join(_TESTDATA_DIRECTORY, filename) | ||
|
|
||
| if not os.path.exists(file_path): | ||
| if overwrite: | ||
| raise FileNotFoundError(f"The file {filename} was not found in the test_data directory.") | ||
| file_path = _get_test_data_path(filename, overwrite=True) | ||
|
|
||
| return file_path | ||
|
|
||
| return _get_test_data_path | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why <1.15.0 ? not great to keep that. If really needed, please add an issue to correct it in another PR. This can be difficult with other packages in time. I prefer scipy >=, !=1.15.0 . it allows that next scipy release automatic upgrade and test if bugs are resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#677