-
Notifications
You must be signed in to change notification settings - Fork 56
Compare GeoJSON assets in job result tests #913
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| from typing import List, Optional, Union | ||
|
|
||
| import numpy | ||
| import pandas | ||
| import xarray | ||
| import xarray.testing | ||
| from xarray import DataArray | ||
|
|
@@ -95,6 +96,55 @@ def _as_xarray_dataarray(data: Union[str, Path, xarray.DataArray]) -> xarray.Dat | |
| return data | ||
|
|
||
|
|
||
| def _load_geodataframe(path: Union[str, Path]): | ||
| """Load a vector data file as a GeoPandas GeoDataFrame.""" | ||
| try: | ||
| import geopandas | ||
| except ImportError as e: | ||
| raise ImportError("Comparing vector data requires the 'geopandas' dependency.") from e | ||
| return geopandas.read_file(path) | ||
|
|
||
|
|
||
| def _compare_geodataframes( | ||
| actual: Union[str, Path], | ||
| expected: Union[str, Path], | ||
| *, | ||
| rtol: float = _DEFAULT_RTOL, | ||
| atol: float = _DEFAULT_ATOL, | ||
| ) -> List[str]: | ||
| """Compare vector geometries and their attribute data.""" | ||
| actual_gdf = _load_geodataframe(actual) | ||
| expected_gdf = _load_geodataframe(expected) | ||
| import geopandas.testing | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. avoid local import when not necessary |
||
|
|
||
| issues = [] | ||
|
|
||
| try: | ||
| pandas.testing.assert_frame_equal( | ||
| actual_gdf.drop(columns=actual_gdf.geometry.name), | ||
| expected_gdf.drop(columns=expected_gdf.geometry.name), | ||
| check_dtype=False, | ||
| check_like=True, | ||
| rtol=rtol, | ||
| atol=atol, | ||
| ) | ||
| except AssertionError as e: | ||
| issues.append(f"Attribute data mismatch:\n{str(e).strip()}") | ||
|
|
||
| try: | ||
| geopandas.testing.assert_geoseries_equal( | ||
| actual_gdf.geometry, | ||
| expected_gdf.geometry, | ||
| check_dtype=False, | ||
| check_geom_type=True, | ||
| check_crs=True, | ||
| ) | ||
| except AssertionError as e: | ||
| issues.append(f"Geometry data mismatch:\n{str(e).strip()}") | ||
|
|
||
| return issues | ||
|
|
||
|
|
||
| def _ascii_art( | ||
| diff_data: DataArray, | ||
| *, | ||
|
|
@@ -459,6 +509,11 @@ def _compare_job_results( | |
| if issues: | ||
| all_issues.append(f"Issues for file {filename!r}:") | ||
| all_issues.extend(issues) | ||
| elif expected_path.suffix.lower() == ".geojson": | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure having ".geojson" is enough as an indicator for being loadable as geodataframe. |
||
| issues = _compare_geodataframes(actual=actual_path, expected=expected_path, rtol=rtol, atol=atol) | ||
| if issues: | ||
| all_issues.append(f"Issues for file {filename!r}:") | ||
| all_issues.extend(issues) | ||
| else: | ||
| _log.warning(f"Unhandled job result asset {filename!r}") | ||
|
|
||
|
|
||
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 is this? geopandas is a base requirement, not an extra,
no need for this guard