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 cache_filename parameter to the NWIS Client constructor #176

Merged
merged 11 commits into from Feb 9, 2022

Conversation

aaraney
Copy link
Member

@aaraney aaraney commented Feb 8, 2022

This PR adds cache_filename parameter to the NWIS Client. This allows a user to specify the location to store cached requests.

closes #175.

Additions

  • cache_filename parameter added to nwis_client.IVDataService. Allows a user to configure the name and location of the sqlite cache. Default remains nwisiv_cache.sqlite. cache_filename's that do not include the suffix .sqlitewill have.sqlite` concatenated as their file-type suffix.

Changes

  • alteration to goal set out in 174. Now, nwis_client > 3.1 will raise RuntimeError exception instead of RuntimeWarning when arguments in **params case insensitively match a non-**params argument. (i.e. service.get(startdt="2022-01-01", ...) the correct call is service.get(startDT="2022-01-01", ...)). Previously, the proposed warning to error timeline was nwis_client > 3.0.

Testing

  1. Unit / integration tests for nwis_client now use temporary cache file. This results in test speeds that are slower, but it mitigates potentially strange behavior that might result from unit tests sharing a cache.

Notes

  • In the future, I might add the ability to pass a hydrotools._restclient.RestClient instance to the IVDataService constructor. it might be nice to have dependency injection when we are to expand to other nwis REST services (i.e. daily value service).

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 ➡️

@aaraney aaraney changed the title Add 175 Add cache_filename parameter to the NWIS Client constructor Feb 8, 2022
@amaes3owp
Copy link

Nice! thank you @aaraney

@jarq6c jarq6c self-requested a review February 9, 2022 18:08
@jarq6c jarq6c added the enhancement New feature or request label Feb 9, 2022
Copy link
Collaborator

@jarq6c jarq6c 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!

@jarq6c jarq6c merged commit d77078b into NOAA-OWP:main Feb 9, 2022
@aaraney aaraney deleted the add_175 branch May 19, 2023 01:24
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.

NWIS Client: Expose cache filename as option
3 participants