DOI-USGS / dataretrieval-python

Python package for retrieving water data from USGS or the multi-agency Water Quality Portal
https://doi-usgs.github.io/dataretrieval-python/
Other
156 stars 37 forks source link

[#135] NLDI data retrieval #136

Closed pkdash closed 4 months ago

pkdash commented 6 months ago

Created this draft PR to get some early feedback.

This is a draft implementation of 2 functions for accessing NLDI. This implementation adds a new dependency - geopandas NLDI API documentation does not help much in terms of implementing validation for input parameters. The parameters of the 2 functions that I have implemented probably need more accurate description to make it user friendly.

thodson-usgs commented 6 months ago

😮

gdf = gpd.GeoDataFrame.from_features(feature_collection)

...slick. Thank you for showing me that one.

Before the final merge, I want to check the size of the dependencies with GeoPandas, and we might discuss with Jeff whether to make it an optional dependency. A nice feature of dataretrieval is that our dependencies are small enough to deploy on Lambda.

Looks great so far.

pkdash commented 6 months ago

@thodson-usgs I like the idea of making the GeoPandas as optional dependency. I am going to implement the remaining NLDI data retrieval functions.

thodson-usgs commented 5 months ago

@pkdash, did you run out of time? Should I pick this up?

pkdash commented 5 months ago

@thodson-usgs Thanks for checking. Yeah has been busy with other projects. I will try to finish it next week. If I can't get to it next week, you can pick this up. I will let you know.

thodson-usgs commented 4 months ago

Ah sorry! I'm too used to relying on CI. I should've tested before mucking up your PR. Stand by

thodson-usgs commented 4 months ago

@pkdash, I tweaked the CI and build pipeline. I might also tweak how we handle geopandas as an optional dependency. Otherwise, these changes are looking good so far. Is this PR ready or still draft?

pkdash commented 4 months ago

@thodson-usgs The only thing left is handling of the geopandas as optional dependency. I ran out of time yesterday. If you can take care of that then the PR is ready. Thanks.