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

Add minimal unit handling to nwm_client #196

Merged
merged 20 commits into from
Jun 3, 2022
Merged

Conversation

jarq6c
Copy link
Collaborator

@jarq6c jarq6c commented Jun 2, 2022

This PR adds minimal unit handling to nwm_client. This is an incremental improvement that includes some housekeeping. Future enhancements will unify the gcp and http interfaces similar to nwm_client_new, and transition from google-cloud-storage to fsspec libraries.

@aaraney may be interested.

Additions

  • unit_system parameter. Setting this parameter to "US" will convert dataframe values returned by get to cubic feet per second and update measurement_unit to "ft^3/s".
  • UnitHandler class with methods to convert units.

Removals

  • The old deprecated gcp_client has been removed.

Changes

  • The gcp target for pip has been removed and google-cloud-storage is brought in automatically.

Testing

  1. Value conversions are tested in test_units.py
  2. pandas.Series conversions are tested in test_gcp.py

Notes

Todos

  • Unify gcp and http interfaces.

Checklist

  • PR has an informative and human-readable title
  • PR is well outlined and documented. See #12 for an example
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (see CONTRIBUTING.md)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output) using numpy docstring formatting
  • Placeholder code is flagged / future todos are captured in comments
  • Reviewers requested with the Reviewers tool ➡️

@jarq6c jarq6c added the enhancement New feature or request label Jun 2, 2022
@jarq6c jarq6c requested a review from hellkite500 June 2, 2022 19:32
@jarq6c jarq6c self-assigned this Jun 2, 2022
@jarq6c
Copy link
Collaborator Author

jarq6c commented Jun 2, 2022

@hellkite500 sorry for the large number of files, I removed the gcp_client which is outdated. The main files with the new functionality are UnitHandler.py, gcp.py, and their associated test files.

Copy link
Member

@hellkite500 hellkite500 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 as is. Just had some thoughts on the implementation of convert_values to ponder.

@jarq6c jarq6c merged commit a131010 into NOAA-OWP:main Jun 3, 2022
@jarq6c jarq6c deleted the nwm-units branch June 3, 2022 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate unit of measurement handling
2 participants