Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/issue 59: Refactor to use GeoPandas GeoDataFrame #96

Merged
merged 10 commits into from
Mar 18, 2024

Conversation

nikki-t
Copy link
Collaborator

@nikki-t nikki-t commented Mar 4, 2024

Github Issue: #59

Description

Optimize the API controller so that it uses GeoPandas GeoDataFrame to format GeoJSON and CSV output response.

Overview of work done

The timeseries API module was modified to store DynamoDB query results as a GeoDataFrame. The GeoDataFrame is used to format either the GeoJSON or CSV output response.

Assumes that reach geometry will be "LineString" and node geometry will be "Point".

API response modifications

  • id key for each element in the features list for GeoJSON response.
  • LineString results are enclosed in double quotation marks for CSV response.

Overview of verification done

Existing API unit tests were updated to account for change in GeoJSON and CSV results response. Two new unit tests were created to test the creation of reach- and node-level GeoDataFrame objects.

Overview of integration done

Issue branch was deployed to SIT and several requests were made to capture responses that test new and existing functionality of the API.

GeoJSON response

{"status":"200 OK","time":397.449,"hits":3,"results":{"csv":"","geojson":{"type":"FeatureCollection","features":[{"id":"0","type":"Feature","properties":{"reach_id":"73282400171","time_str":"2023-07-28T22:10:37Z","wse":"211.178","slope":"-0.0007516914999999999"},"geometry":{}},{"id":"1","type":"Feature","properties":{"reach_id":"73282400171","time_str":"2023-09-08T15:40:49Z","wse":"82.082","slope":"-1.0184e-06"},"geometry":{}},{"id":"2","type":"Feature","properties":{"reach_id":"73282400171","time_str":"2023-09-29T12:25:52Z","wse":"206.49349999999998","slope":"-0.0007833485000000001"},"geometry":{}}]}}}

GeoJSON response with geometry

{"status":"200 OK","time":458.179,"hits":3,"results":{"csv":"","geojson":{"type":"FeatureCollection","features":[{"id":"0","type":"Feature","properties":{"reach_id":"73282400171","time_str":"2023-07-28T22:10:37Z","wse":"211.178","slope":"-0.0007516914999999999"},"geometry":{"type":"LineString","coordinates":[[-88.300727,33.252014],[-88.300602,33.252286],[-88.300541,33.252557],[-88.300481,33.252828],[-88.300421,33.253099],[-88.30036,33.25337],[-88.3003,33.253642],[-88.300239,33.253913],[-88.300179,33.254184],[-88.300119,33.254455],[-88.300058,33.254727],[-88.299998,33.254998],[-88.299938,33.255269],[-88.299942,33.255539],[-88.299946,33.25581],[-88.29995,33.256081],[-88.300082,33.25635],[-88.300215,33.256619],[-88.300284,33.256889],[-88.300416,33.257158],[-88.300549,33.257427],[-88.300682,33.257696],[-88.300879,33.257965],[-88.30114,33.258179],[-88.301401,33.258392],[-88.301725,33.258551],[-88.302049,33.258656],[-88.302372,33.258707],[-88.302694,33.258758],[-88.303016,33.258754],[-88.303339,33.258805],[-88.303662,33.258856],[-88.303985,33.258906],[-88.304308,33.258957],[-88.30463,33.259008],[-88.304953,33.259059],[-88.305276,33.259109],[-88.305599,33.25916],[-88.305922,33.259211],[-88.306245,33.259261]]}},{"id":"1","type":"Feature","properties":{"reach_id":"73282400171","time_str":"2023-09-08T15:40:49Z","wse":"82.082","slope":"-1.0184e-06"},"geometry":{"type":"LineString","coordinates":[[-88.300727,33.252014],[-88.300602,33.252286],[-88.300541,33.252557],[-88.300481,33.252828],[-88.300421,33.253099],[-88.30036,33.25337],[-88.3003,33.253642],[-88.300239,33.253913],[-88.300179,33.254184],[-88.300119,33.254455],[-88.300058,33.254727],[-88.299998,33.254998],[-88.299938,33.255269],[-88.299942,33.255539],[-88.299946,33.25581],[-88.29995,33.256081],[-88.300082,33.25635],[-88.300215,33.256619],[-88.300284,33.256889],[-88.300416,33.257158],[-88.300549,33.257427],[-88.300682,33.257696],[-88.300879,33.257965],[-88.30114,33.258179],[-88.301401,33.258392],[-88.301725,33.258551],[-88.302049,33.258656],[-88.302372,33.258707],[-88.302694,33.258758],[-88.303016,33.258754],[-88.303339,33.258805],[-88.303662,33.258856],[-88.303985,33.258906],[-88.304308,33.258957],[-88.30463,33.259008],[-88.304953,33.259059],[-88.305276,33.259109],[-88.305599,33.25916],[-88.305922,33.259211],[-88.306245,33.259261]]}},{"id":"2","type":"Feature","properties":{"reach_id":"73282400171","time_str":"2023-09-29T12:25:52Z","wse":"206.49349999999998","slope":"-0.0007833485000000001"},"geometry":{"type":"LineString","coordinates":[[-88.300727,33.252014],[-88.300602,33.252286],[-88.300541,33.252557],[-88.300481,33.252828],[-88.300421,33.253099],[-88.30036,33.25337],[-88.3003,33.253642],[-88.300239,33.253913],[-88.300179,33.254184],[-88.300119,33.254455],[-88.300058,33.254727],[-88.299998,33.254998],[-88.299938,33.255269],[-88.299942,33.255539],[-88.299946,33.25581],[-88.29995,33.256081],[-88.300082,33.25635],[-88.300215,33.256619],[-88.300284,33.256889],[-88.300416,33.257158],[-88.300549,33.257427],[-88.300682,33.257696],[-88.300879,33.257965],[-88.30114,33.258179],[-88.301401,33.258392],[-88.301725,33.258551],[-88.302049,33.258656],[-88.302372,33.258707],[-88.302694,33.258758],[-88.303016,33.258754],[-88.303339,33.258805],[-88.303662,33.258856],[-88.303985,33.258906],[-88.304308,33.258957],[-88.30463,33.259008],[-88.304953,33.259059],[-88.305276,33.259109],[-88.305599,33.25916],[-88.305922,33.259211],[-88.306245,33.259261]]}}]}}}

CSV response with geometry

{"status":"200 OK","time":458.004,"hits":3,"results":{"csv":"reach_id,time_str,wse,width,geometry\n73282400171,2023-07-28T22:10:37Z,211.178,5271.091685,\"LINESTRING (-88.300727 33.252014, -88.300602 33.252286, -88.300541 33.252557, -88.300481 33.252828, -88.300421 33.253099, -88.30036 33.25337, -88.3003 33.253642, -88.300239 33.253913, -88.300179 33.254184, -88.300119 33.254455, -88.300058 33.254727, -88.299998 33.254998, -88.299938 33.255269, -88.299942 33.255539, -88.299946 33.25581, -88.29995 33.256081, -88.300082 33.25635, -88.300215 33.256619, -88.300284 33.256889, -88.300416 33.257158, -88.300549 33.257427, -88.300682 33.257696, -88.300879 33.257965, -88.30114 33.258179, -88.301401 33.258392, -88.301725 33.258551, -88.302049 33.258656, -88.302372 33.258707, -88.302694 33.258758, -88.303016 33.258754, -88.303339 33.258805, -88.303662 33.258856, -88.303985 33.258906, -88.304308 33.258957, -88.30463 33.259008, -88.304953 33.259059, -88.305276 33.259109, -88.305599 33.25916, -88.305922 33.259211, -88.306245 33.259261)\"\n73282400171,2023-09-08T15:40:49Z,82.082,2655.230594,\"LINESTRING (-88.300727 33.252014, -88.300602 33.252286, -88.300541 33.252557, -88.300481 33.252828, -88.300421 33.253099, -88.30036 33.25337, -88.3003 33.253642, -88.300239 33.253913, -88.300179 33.254184, -88.300119 33.254455, -88.300058 33.254727, -88.299998 33.254998, -88.299938 33.255269, -88.299942 33.255539, -88.299946 33.25581, -88.29995 33.256081, -88.300082 33.25635, -88.300215 33.256619, -88.300284 33.256889, -88.300416 33.257158, -88.300549 33.257427, -88.300682 33.257696, -88.300879 33.257965, -88.30114 33.258179, -88.301401 33.258392, -88.301725 33.258551, -88.302049 33.258656, -88.302372 33.258707, -88.302694 33.258758, -88.303016 33.258754, -88.303339 33.258805, -88.303662 33.258856, -88.303985 33.258906, -88.304308 33.258957, -88.30463 33.259008, -88.304953 33.259059, -88.305276 33.259109, -88.305599 33.25916, -88.305922 33.259211, -88.306245 33.259261)\"\n73282400171,2023-09-29T12:25:52Z,206.49349999999998,5404.127035,\"LINESTRING (-88.300727 33.252014, -88.300602 33.252286, -88.300541 33.252557, -88.300481 33.252828, -88.300421 33.253099, -88.30036 33.25337, -88.3003 33.253642, -88.300239 33.253913, -88.300179 33.254184, -88.300119 33.254455, -88.300058 33.254727, -88.299998 33.254998, -88.299938 33.255269, -88.299942 33.255539, -88.299946 33.25581, -88.29995 33.256081, -88.300082 33.25635, -88.300215 33.256619, -88.300284 33.256889, -88.300416 33.257158, -88.300549 33.257427, -88.300682 33.257696, -88.300879 33.257965, -88.30114 33.258179, -88.301401 33.258392, -88.301725 33.258551, -88.302049 33.258656, -88.302372 33.258707, -88.302694 33.258758, -88.303016 33.258754, -88.303339 33.258805, -88.303662 33.258856, -88.303985 33.258906, -88.304308 33.258957, -88.30463 33.259008, -88.304953 33.259059, -88.305276 33.259109, -88.305599 33.25916, -88.305922 33.259211, -88.306245 33.259261)\"\n","geojson":{}}}

Empty request

{"error" : "400: This required parameter is missing: 'feature'"}

Request failure (Not all request fields are in SWOT shapefiles)

{"error" : "400: fields parameter should contain valid SWOT fields"}                                                                                                                                                         

PR checklist:

  • Linted
  • Updated unit tests
  • Updated changelog
  • Integration testing

See Pull Request Review Checklist for pointers on reviewing this pull request

@nikki-t nikki-t requested review from torimcd and frankinspace March 4, 2024 16:59
@nikki-t nikki-t self-assigned this Mar 4, 2024
@frankinspace frankinspace changed the title Feature/issue 59 Feature/issue 59: Refactor to use GeoPandas GeoDataFrame Mar 5, 2024
Copy link
Member

@frankinspace frankinspace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a quick changelog entry

@nikki-t
Copy link
Collaborator Author

nikki-t commented Mar 5, 2024

@frankinspace - Changelog entry added!

@nikki-t
Copy link
Collaborator Author

nikki-t commented Mar 6, 2024

Thinking about the discussion around issue #100, the GeoJSON response will return a response without geometry if the request does not includegeometry in the fields parameter. This was replicating the previous behavior before implementing the use of the GeoDataFrame. See these lines of code.

Instead, does it make sense to always return the geometry for a GeoJSON response regardless of what is present in the request parameter fields so that it is always returning valid GeoJSON? If so, I would like to make the change before merging the PR.

@frankinspace
Copy link
Member

Ah I see, yes it should always return geometry because it is required by the geojson specification: https://datatracker.ietf.org/doc/html/rfc7946#section-3.2

Speaking of, could we also add a unit test that validates the output of a geojson request against the geojson schema? That way we can be sure we stay in compliance with the specification.

@nikki-t
Copy link
Collaborator Author

nikki-t commented Mar 7, 2024

Modified the timeseries Lamdba so that is always returns geometry for GeoJSON responses and created a new unit test to validate GeoJSON.

Repeated SIT tests to confirm functionality.

GeoJSON reach response

{"status":"200 OK","time":2046.334,"hits":3,"results":{"csv":"","geojson":{"type":"FeatureCollection","features":[{"id":"0","type":"Feature","properties":{"reach_id":"73282400171","time_str":"2023-07-28T22:10:37Z","wse":"211.178","slope":"-0.0007516914999999999"},"geometry":{"type":"LineString","coordinates":[[-88.300727,33.252014],[-88.300602,33.252286],[-88.300541,33.252557],[-88.300481,33.252828],[-88.300421,33.253099],[-88.30036,33.25337],[-88.3003,33.253642],[-88.300239,33.253913],[-88.300179,33.254184],[-88.300119,33.254455],[-88.300058,33.254727],[-88.299998,33.254998],[-88.299938,33.255269],[-88.299942,33.255539],[-88.299946,33.25581],[-88.29995,33.256081],[-88.300082,33.25635],[-88.300215,33.256619],[-88.300284,33.256889],[-88.300416,33.257158],[-88.300549,33.257427],[-88.300682,33.257696],[-88.300879,33.257965],[-88.30114,33.258179],[-88.301401,33.258392],[-88.301725,33.258551],[-88.302049,33.258656],[-88.302372,33.258707],[-88.302694,33.258758],[-88.303016,33.258754],[-88.303339,33.258805],[-88.303662,33.258856],[-88.303985,33.258906],[-88.304308,33.258957],[-88.30463,33.259008],[-88.304953,33.259059],[-88.305276,33.259109],[-88.305599,33.25916],[-88.305922,33.259211],[-88.306245,33.259261]]}},{"id":"1","type":"Feature","properties":{"reach_id":"73282400171","time_str":"2023-09-08T15:40:49Z","wse":"82.082","slope":"-1.0184e-06"},"geometry":{"type":"LineString","coordinates":[[-88.300727,33.252014],[-88.300602,33.252286],[-88.300541,33.252557],[-88.300481,33.252828],[-88.300421,33.253099],[-88.30036,33.25337],[-88.3003,33.253642],[-88.300239,33.253913],[-88.300179,33.254184],[-88.300119,33.254455],[-88.300058,33.254727],[-88.299998,33.254998],[-88.299938,33.255269],[-88.299942,33.255539],[-88.299946,33.25581],[-88.29995,33.256081],[-88.300082,33.25635],[-88.300215,33.256619],[-88.300284,33.256889],[-88.300416,33.257158],[-88.300549,33.257427],[-88.300682,33.257696],[-88.300879,33.257965],[-88.30114,33.258179],[-88.301401,33.258392],[-88.301725,33.258551],[-88.302049,33.258656],[-88.302372,33.258707],[-88.302694,33.258758],[-88.303016,33.258754],[-88.303339,33.258805],[-88.303662,33.258856],[-88.303985,33.258906],[-88.304308,33.258957],[-88.30463,33.259008],[-88.304953,33.259059],[-88.305276,33.259109],[-88.305599,33.25916],[-88.305922,33.259211],[-88.306245,33.259261]]}},{"id":"2","type":"Feature","properties":{"reach_id":"73282400171","time_str":"2023-09-29T12:25:52Z","wse":"206.49349999999998","slope":"-0.0007833485000000001"},"geometry":{"type":"LineString","coordinates":[[-88.300727,33.252014],[-88.300602,33.252286],[-88.300541,33.252557],[-88.300481,33.252828],[-88.300421,33.253099],[-88.30036,33.25337],[-88.3003,33.253642],[-88.300239,33.253913],[-88.300179,33.254184],[-88.300119,33.254455],[-88.300058,33.254727],[-88.299998,33.254998],[-88.299938,33.255269],[-88.299942,33.255539],[-88.299946,33.25581],[-88.29995,33.256081],[-88.300082,33.25635],[-88.300215,33.256619],[-88.300284,33.256889],[-88.300416,33.257158],[-88.300549,33.257427],[-88.300682,33.257696],[-88.300879,33.257965],[-88.30114,33.258179],[-88.301401,33.258392],[-88.301725,33.258551],[-88.302049,33.258656],[-88.302372,33.258707],[-88.302694,33.258758],[-88.303016,33.258754],[-88.303339,33.258805],[-88.303662,33.258856],[-88.303985,33.258906],[-88.304308,33.258957],[-88.30463,33.259008],[-88.304953,33.259059],[-88.305276,33.259109],[-88.305599,33.25916],[-88.305922,33.259211],[-88.306245,33.259261]]}}]}}}

CSV reach response

{"status":"200 OK","time":592.921,"hits":3,"results":{"csv":"reach_id,time_str,wse,width\n73282400171,2023-07-28T22:10:37Z,211.178,5271.091685\n73282400171,2023-09-08T15:40:49Z,82.082,2655.230594\n73282400171,2023-09-29T12:25:52Z,206.49349999999998,5404.127035\n","geojson":{}}}

Request with missing parameters

{"error" : "400: This required parameter is missing: 'feature'"}

Request GeoJSON failure (not all fields are in swot shapefile)

{"error" : "400: fields parameter should contain valid SWOT fields"}

Note: There are warning generated with the use of GeoPandas and are visible in pytest output. It looks these warnings are being addressed in this issue.

Should be all set to merge the PR.

Copy link
Collaborator

@torimcd torimcd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@frankinspace frankinspace merged commit 33c741f into develop Mar 18, 2024
1 check passed
@frankinspace frankinspace deleted the feature/issue-59 branch March 18, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants