Validate spatial files referenced in a CSV column #1504

emlys · 2024-01-18T18:04:37Z

Description

Fixes #327
Currently limited to validating rasters and vectors as the issue specified, but it would be simple to expand to all types of columns.

Checklist

Updated HISTORY.rst and link to any relevant issue (if these changes are user-facing)
Updated the user's guide (if needed)
Tested the Workbench UI (if relevant)

emlys · 2024-01-22T18:45:39Z

src/natcap/invest/coastal_vulnerability.py

@@ -3449,36 +3447,6 @@ def logger_callback(proportion_complete):
    return logger_callback


-def _validate_habitat_table_paths(habitat_table_path):


This is now redundant to what's done in validation by default.

emlys · 2024-01-22T18:46:28Z

src/natcap/invest/habitat_quality.py

@@ -1044,23 +1044,13 @@ def _validate_threat_path(threat_path, lulc_key):
    """
    # Checking threat path exists to control custom error messages
    # for user readability.
-    try:
-        threat_gis_type = pygeoprocessing.get_gis_type(threat_path)


No longer need to check that the file can be opened by GDAL. Validation already does that

For this model there are three columns expecting raster paths. Only the "cur_path" column is required. The "base_path", and "fut_path" columns are conditionally required. If the user has an empty "base_path" column with the conditional unmet (the column is not required but is present in the table), I think the model says that it's okay for that column to be empty.

Does that still work here with validation?

Okay, I see that this check is still left in for the model to look for.

So I guess my question is, does validate check conditional spatial files referenced in a CSV?

Okay, I see that in validation for this an empty entry is returned with no error.

Yes, empty entries are passed through and don't raise any validation warning. Note that this applies even when the column is required - NA values don't fail validation because in some cases we do need to allow NA values.

emlys · 2024-01-22T18:50:13Z

tests/test_coastal_vulnerability.py

@@ -1548,21 +1548,6 @@ def test_clip_project_vector_on_invalid_geometry(self):
        n_actual_features = target_layer.GetFeatureCount()
        self.assertTrue(n_actual_features == n_features - 1)

-    def test_invalid_habitat_table_paths(self):


This is now covered by regular validation and validation tests.

dcdenu4

Thanks @emlys for this. I had some small comments / suggestions / questions. Looking at the PR checklist reminder, I was also curious if you tested this in the UI at all?

dcdenu4 · 2024-01-25T14:38:52Z

src/natcap/invest/habitat_quality.py

@@ -1044,23 +1044,13 @@ def _validate_threat_path(threat_path, lulc_key):
    """
    # Checking threat path exists to control custom error messages
    # for user readability.
-    try:
-        threat_gis_type = pygeoprocessing.get_gis_type(threat_path)


For this model there are three columns expecting raster paths. Only the "cur_path" column is required. The "base_path", and "fut_path" columns are conditionally required. If the user has an empty "base_path" column with the conditional unmet (the column is not required but is present in the table), I think the model says that it's okay for that column to be empty.

Does that still work here with validation?

dcdenu4 · 2024-01-25T14:42:31Z

src/natcap/invest/habitat_quality.py

@@ -1044,23 +1044,13 @@ def _validate_threat_path(threat_path, lulc_key):
    """
    # Checking threat path exists to control custom error messages
    # for user readability.
-    try:
-        threat_gis_type = pygeoprocessing.get_gis_type(threat_path)


Okay, I see that this check is still left in for the model to look for.

So I guess my question is, does validate check conditional spatial files referenced in a CSV?

dcdenu4 · 2024-01-25T14:45:35Z

src/natcap/invest/hra.py

@@ -80,7 +80,7 @@
                            "values besides 0 or 1 will be treated as 0.")
                    }},
                    "fields": {},
-                    "geometries": spec_utils.POLYGONS,
+                    "geometries": spec_utils.ALL_GEOMS,


If this is true that multiple geometry types are allowed for this, should the "about" section below be updated as well? It currently states:

For vectors, a polygon indicates...

Sure! I had to update this line because one of our tests used a non-polygon geometry, which would now fail validation.

dcdenu4 · 2024-01-26T20:19:36Z

src/natcap/invest/habitat_quality.py

@@ -1044,23 +1044,13 @@ def _validate_threat_path(threat_path, lulc_key):
    """
    # Checking threat path exists to control custom error messages
    # for user readability.
-    try:
-        threat_gis_type = pygeoprocessing.get_gis_type(threat_path)


Okay, I see that in validation for this an empty entry is returned with no error.

tests/test_validation.py

dcdenu4 · 2024-01-26T20:38:25Z

tests/test_validation.py

@@ -1352,6 +1342,43 @@ def test_rows(self):
        self.assertEqual(df['row2'][1], 3)
        self.assertEqual(df['row2'].dtype, float)

+    def test_recursive_path_validation(self):


Should there also be a test case for vector paths?

Sure! I added a couple more tests

Co-authored-by: Doug <[email protected]>

dcdenu4

Thanks Emily! I think this is looking good!

dcdenu4 · 2024-03-20T17:37:48Z

Hey @emlys, there appears to be a Windows Workbench test that fails consistently.

dcdenu4

Just removing approval, so we can check out that test failure.

Test failing.

dcdenu4

I just tried for the first time, "dismiss review".

emlys · 2024-03-28T18:15:26Z

@dcdenu4 , I'm not sure what was happening with that test, but it's passing now, so it must've just been flaky. I'll go ahead and merge if you approve!

dcdenu4

Thanks!

emlys added 8 commits November 16, 2023 15:04

work in progress: validate nested types in csv

128abfe

Merge branch 'main' into feature/327

2daf974

update csv validation tests

671764f

work in progress: updating tests

0152fba

update hra tests to work with nested path validation

66fb9b8

update rec model tests to pass new csv path validation

d63437d

update HQ model and tests for new csv path validation

5ca1ec3

update tests

a5cd718

emlys commented Jan 22, 2024

View reviewed changes

emlys added 5 commits January 22, 2024 11:23

clean up

2579a47

allow any geom type in hra stressor vector

ea7a4a3

restore os.sep separator in validation tests

91ebedb

Merge branch 'main' into feature/327

fee390f

add history note

0d0b05e

emlys changed the title ~~Feature/327~~ Validate files referenced in a CSV column Jan 24, 2024

limit recursive csv validation to spatial types

14d78ed

emlys changed the title ~~Validate files referenced in a CSV column~~ Validate spatial files referenced in a CSV column Jan 24, 2024

emlys requested a review from dcdenu4 January 24, 2024 20:06

emlys self-assigned this Jan 24, 2024

emlys marked this pull request as ready for review January 24, 2024 20:07

dcdenu4 requested changes Jan 26, 2024

View reviewed changes

emlys and others added 5 commits January 26, 2024 13:02

Update tests/test_validation.py

1d1b849

Co-authored-by: Doug <[email protected]>

Update tests/test_validation.py

c35f97b

Co-authored-by: Doug <[email protected]>

Merge branch 'main' into feature/327

d8f8458

expand recursive validation tests

2b0eafc

correct HRA info table docs: vector can have any geom

2ba7286

emlys requested a review from dcdenu4 March 12, 2024 21:35

dcdenu4 previously approved these changes Mar 20, 2024

View reviewed changes

dcdenu4 self-requested a review March 20, 2024 17:37

dcdenu4 reviewed Mar 20, 2024

View reviewed changes

dcdenu4 self-requested a review March 20, 2024 17:39

dcdenu4 reviewed Mar 20, 2024

View reviewed changes

emlys requested a review from dcdenu4 March 28, 2024 18:16

dcdenu4 enabled auto-merge March 29, 2024 18:14

dcdenu4 approved these changes Mar 29, 2024

View reviewed changes

dcdenu4 merged commit da55458 into natcap:main Mar 29, 2024
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate spatial files referenced in a CSV column #1504

Validate spatial files referenced in a CSV column #1504

emlys commented Jan 18, 2024 •

edited

Loading

emlys Jan 22, 2024

emlys Jan 22, 2024

dcdenu4 Jan 25, 2024

dcdenu4 Jan 25, 2024

dcdenu4 Jan 26, 2024

emlys Mar 12, 2024 •

edited

Loading

emlys Jan 22, 2024

dcdenu4 left a comment

dcdenu4 Jan 25, 2024

dcdenu4 Jan 25, 2024

dcdenu4 Jan 25, 2024

emlys Mar 12, 2024

dcdenu4 Jan 26, 2024

dcdenu4 Jan 26, 2024

emlys Mar 12, 2024

dcdenu4 left a comment

dcdenu4 commented Mar 20, 2024

dcdenu4 left a comment

dcdenu4 left a comment

emlys commented Mar 28, 2024 •

edited

Loading

dcdenu4 left a comment

		@@ -3449,36 +3447,6 @@ def logger_callback(proportion_complete):
		return logger_callback


		def _validate_habitat_table_paths(habitat_table_path):

Validate spatial files referenced in a CSV column #1504

Validate spatial files referenced in a CSV column #1504

Conversation

emlys commented Jan 18, 2024 • edited Loading

Description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emlys Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dcdenu4 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dcdenu4 left a comment

Choose a reason for hiding this comment

dcdenu4 commented Mar 20, 2024

dcdenu4 left a comment

Choose a reason for hiding this comment

dcdenu4 left a comment

Choose a reason for hiding this comment

emlys commented Mar 28, 2024 • edited Loading

dcdenu4 left a comment

Choose a reason for hiding this comment

emlys commented Jan 18, 2024 •

edited

Loading

emlys Mar 12, 2024 •

edited

Loading

emlys commented Mar 28, 2024 •

edited

Loading