[#135] NLDI data retrieval #136
Conversation
|
😮 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 Looks great so far. |
|
@thodson-usgs I like the idea of making the GeoPandas as optional dependency. I am going to implement the remaining NLDI data retrieval functions. |
|
@pkdash, did you run out of time? Should I pick this up? |
|
@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. |
|
Ah sorry! I'm too used to relying on CI. I should've tested before mucking up your PR. Stand by |
thodson-usgs
force-pushed
the
135-nldi-data-retrieval
branch
from
6b4c5c3
to
e7ef9f0
Compare
thodson-usgs
force-pushed
the
135-nldi-data-retrieval
branch
from
e7ef9f0
to
a5b2ec8
Compare
|
@pkdash, I tweaked the CI and build pipeline. I might also tweak how we handle geopandas as an optional dependency. |
|
@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. |
thodson-usgs
force-pushed
the
135-nldi-data-retrieval
branch
from
7f93af8
to
2908373
Compare
There was a problem hiding this comment.
Great work @pkdash! I think I will ultimately rewrite search, but otherwise this PR looks good to go.
| return gdf | ||
|
|
||
|
|
||
| def search( |
There was a problem hiding this comment.
I intend search to be a lower-level function to return a nested dict of multiple feature types. For example, if we wanted to get multiple features upstream of a site, this would allow us to accomplish that in a single API call. Nevertheless, I'm fine with this for the first cut. Great work!
There was a problem hiding this comment.
Due to my lack of understanding of the domain, I was struggling a bit to implement the search functionality.
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.