Xcube server STAC API now returns a conformant collection time extent #1080

konstntokas · 2024-10-07T13:45:55Z

Checklist:

Add unit tests and/or doctests in docstrings
~~Add docstrings and API docs for any new/modified user-facing classes and functions~~
~~New/modified features documented in docs/source/*~~
Changes documented in CHANGES.md
GitHub CI passes
AppVeyor CI passes
Test coverage remains or increases (target 100%)

konstntokas · 2024-10-07T14:27:54Z

test/webapi/ows/stac/test_controllers.py

@@ -148,6 +148,16 @@
 }


+def assert_equal_deep_dict(self, dict1, dict2):


I add this function to test for nested dictionaries.

Ok. However, you should keep track of the path and output it as part of the error message.

But actually it is strange why this is needed. TestCase.assertEquals() actually does a deep equality comparison.

Ok. However, you should keep track of the path and output it as part of the error message.

done

But actually it is strange why this is needed. TestCase.assertEquals() actually does a deep equality comparison.

TestCase.assertEqual() also checks for the order of the keys in the dictionary, which gives me an error.

konstntokas · 2024-10-07T14:29:28Z

test/webapi/ows/stac/test_controllers.py

@@ -174,7 +184,7 @@ def test_get_datasets_collection_item(self):
            "demo-1w",
        )
        self.assertIsInstance(result, dict)
-        self.assertCountEqual(self.read_json("stac-datacubes-item.json"), result)
+        assert_equal_deep_dict(self, self.read_json("stac-datacubes-item.json"), result)


Previously, it was only tested that the keys in the first layer of the dictionaries are equal. Now I test the full content of the dictionaries.

I was wondering why you define assert_equal_deep_dict rather than just using self.assertEqual(), and eventually realized that you presumably need something that's insensitive to the order of iterables within the dictionaries. I suggest adding a short docstring to assert_equal_deep_dict to explain this.

I was wondering why you define assert_equal_deep_dict rather than just using self.assertEqual(), and eventually realized that you presumably need something that's insensitive to the order of iterables within the dictionaries. I suggest adding a short docstring to assert_equal_deep_dict to explain this.

done

forman

Approved with comments. Please have a look.

forman · 2024-10-07T14:34:46Z

test/webapi/ows/stac/test_controllers.py

@@ -148,6 +148,16 @@
 }


+def assert_equal_deep_dict(self, dict1, dict2):


Ok. However, you should keep track of the path and output it as part of the error message.

forman · 2024-10-07T14:36:02Z

xcube/webapi/ows/stac/controllers.py

@@ -632,7 +632,7 @@ def _get_single_dataset_collection(
                    {"cellsCount": gm.size[1], "resolution": gm.xy_res[1]},
                ],
            },
-            "temporal": {"interval": [[time_interval]], "grid": get_time_grid(dataset)},
+            "temporal": {"interval": [time_interval], "grid": get_time_grid(dataset)},


Aha, the actual fix.

forman · 2024-10-07T14:36:43Z

xcube/util/jsonencoder.py

@@ -102,7 +102,7 @@ def _convert_default(obj: Any) -> Any:
            elif np.issubdtype(obj.dtype, np.floating):
                return float(obj)
            elif np.issubdtype(obj.dtype, np.datetime64):
-                return np.datetime_as_string(obj, timezone="UTC", unit="s")
+                return str(np.datetime_as_string(obj, timezone="UTC", unit="s"))


Please add a comment why this is needed.

forman · 2024-10-07T14:39:08Z

test/webapi/ows/stac/test_controllers.py

@@ -148,6 +148,16 @@
 }


+def assert_equal_deep_dict(self, dict1, dict2):


But actually it is strange why this is needed. TestCase.assertEquals() actually does a deep equality comparison.

pont-us

I had one suggestion (see comment), but approving since I don't need to review again.

pont-us · 2024-10-08T10:42:33Z

test/webapi/ows/stac/test_controllers.py

@@ -174,7 +184,7 @@ def test_get_datasets_collection_item(self):
            "demo-1w",
        )
        self.assertIsInstance(result, dict)
-        self.assertCountEqual(self.read_json("stac-datacubes-item.json"), result)
+        assert_equal_deep_dict(self, self.read_json("stac-datacubes-item.json"), result)


I was wondering why you define assert_equal_deep_dict rather than just using self.assertEqual(), and eventually realized that you presumably need something that's insensitive to the order of iterables within the dictionaries. I suggest adding a short docstring to assert_equal_deep_dict to explain this.

konstntokas added 2 commits October 7, 2024 15:41

addes issue #1077

53295a6

remove test config

f96a11a

konstntokas commented Oct 7, 2024

View reviewed changes

forman requested review from forman and pont-us October 7, 2024 14:31

konstntokas changed the title ~~Konstntokas 1077 stac non conformant collection~~ Xcube server STAC API now returns a conformant collection time extent Oct 7, 2024

forman reviewed Oct 7, 2024

View reviewed changes

Normans review addressed

3d25072

pont-us approved these changes Oct 8, 2024

View reviewed changes

Ponts review addressed

e4ba6c1

konstntokas merged commit 033af87 into main Oct 8, 2024
3 checks passed

konstntokas deleted the konstntokas-1077-stac_non_conformant_collection branch October 8, 2024 13:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Xcube server STAC API now returns a conformant collection time extent #1080

Xcube server STAC API now returns a conformant collection time extent #1080

konstntokas commented Oct 7, 2024 •

edited

Loading

konstntokas Oct 7, 2024

forman Oct 7, 2024

forman Oct 7, 2024

konstntokas Oct 7, 2024

konstntokas Oct 7, 2024

konstntokas Oct 7, 2024

pont-us Oct 8, 2024

konstntokas Oct 8, 2024

forman left a comment

forman Oct 7, 2024

forman Oct 7, 2024

forman Oct 7, 2024

konstntokas Oct 7, 2024

forman Oct 7, 2024

pont-us left a comment

pont-us Oct 8, 2024

		@@ -148,6 +148,16 @@
		}


		def assert_equal_deep_dict(self, dict1, dict2):

Xcube server STAC API now returns a conformant collection time extent #1080

Xcube server STAC API now returns a conformant collection time extent #1080

Conversation

konstntokas commented Oct 7, 2024 • edited Loading

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

forman 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

pont-us left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

konstntokas commented Oct 7, 2024 •

edited

Loading