MAAP-Project / gedi-subsetter

MAAP DPS algorithm for subsetting GEDI data
Apache License 2.0
7 stars 0 forks source link

Use cloud-hosted collections only #66

Closed chuckwondo closed 6 months ago

chuckwondo commented 7 months ago

All supported GEDI collections are now cloud-hosted, which means we can pull granules via S3.

This PR includes the following:

wildintellect commented 7 months ago

I got a lint error, and 1 warning. Should we fix before accepting? Linter caught in /tests/conftest.py:

             self._MAAP_HOST = "api.maap-project.org"
-            self._SEARCH_COLLECTION_URL = f"https://{self._MAAP_HOST}/api/cmr/collections"
+            self._SEARCH_COLLECTION_URL = (
+                f"https://{self._MAAP_HOST}/api/cmr/collections"
+            )
             self._CMR = CMR([], 20, {})
             self.aws = AWS(

Warnings

tests/test_gedi_utils.py::test_subset_hdf5[lat_lowestmode-lon_lowestmode-all-columns0-sensitivity < 0.9-0]
  /home/ochotona/code/devseed/maap/gedi-subsetter/tests/test_gedi_utils.py:222: FutureWarning: <class 'geopandas.array.GeometryArray'>._reduce will require a `keepdims` parameter in the future
    assert gdf.notna().all(axis=None)
chuckwondo commented 7 months ago

I got a lint error, and 1 warning. Should we fix before accepting? Linter caught in /tests/conftest.py:

             self._MAAP_HOST = "api.maap-project.org"
-            self._SEARCH_COLLECTION_URL = f"https://{self._MAAP_HOST}/api/cmr/collections"
+            self._SEARCH_COLLECTION_URL = (
+                f"https://{self._MAAP_HOST}/api/cmr/collections"
+            )
             self._CMR = CMR([], 20, {})
             self.aws = AWS(

Warnings

tests/test_gedi_utils.py::test_subset_hdf5[lat_lowestmode-lon_lowestmode-all-columns0-sensitivity < 0.9-0]
 /home/ochotona/code/devseed/maap/gedi-subsetter/tests/test_gedi_utils.py:222: FutureWarning: <class 'geopandas.array.GeometryArray'>._reduce will require a `keepdims` parameter in the future
   assert gdf.notna().all(axis=None)

Ah, I missed that formatting error because that was a change I pushed from the ADE, where pre-commit didn't catch that. I'll fix it.

Regarding the FutureWarning, that's coming from deep within a package, not something caused directly by our code, so I can't fix that. I could silence the warning, but I'd rather keep it visible for now.

chuckwondo commented 7 months ago

I got a lint error, and 1 warning. Should we fix before accepting? Linter caught in /tests/conftest.py:

             self._MAAP_HOST = "api.maap-project.org"
-            self._SEARCH_COLLECTION_URL = f"https://{self._MAAP_HOST}/api/cmr/collections"
+            self._SEARCH_COLLECTION_URL = (
+                f"https://{self._MAAP_HOST}/api/cmr/collections"
+            )
             self._CMR = CMR([], 20, {})
             self.aws = AWS(

Warnings

tests/test_gedi_utils.py::test_subset_hdf5[lat_lowestmode-lon_lowestmode-all-columns0-sensitivity < 0.9-0]
 /home/ochotona/code/devseed/maap/gedi-subsetter/tests/test_gedi_utils.py:222: FutureWarning: <class 'geopandas.array.GeometryArray'>._reduce will require a `keepdims` parameter in the future
   assert gdf.notna().all(axis=None)

Ah, I missed that formatting error because that was a change I pushed from the ADE, where pre-commit didn't catch that. I'll fix it.

Actually, the build should have failed on that because pre-commit.ci should have been triggered, but it wasn't. Let me see if I can get pre-commit.ci to run as expected.

chuckwondo commented 7 months ago

@wildintellect, I added a blurb on vcrpy and I got pre-commit.ci to trigger. It turned out that there's a setup step that is not clearly indicated on the pre-commit.com site. Essentially, each repo must give permission to pre-commit.ci, in addition to adding a ci block to .pre-commit-config.yaml.

Gah! Looks like pre-commit failed. At least it was triggered! Let me see what happened.

chuckwondo commented 7 months ago

@wildintellect, I added a blurb on vcrpy and I got pre-commit.ci to trigger. It turned out that there's a setup step that is not clearly indicated on the pre-commit.com site. Essentially, each repo must give permission to pre-commit.ci, in addition to adding a ci block to .pre-commit-config.yaml.

Gah! Looks like pre-commit failed. At least it was triggered! Let me see what happened.

Oooh! I see what happened. I have to tweak the pre-commit file. I had added a hook that ends up calling conda, which isn't installed on the pre-commit.ci server. Let me address that.

chuckwondo commented 6 months ago

@wildintellect, I had to wrestle a bit with pre-commit stuff since your earlier approval. Mind having another look?