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

Metrics: Default to numpy scalars for computation #163

Merged
merged 9 commits into from
Dec 9, 2021

Conversation

jarq6c
Copy link
Collaborator

@jarq6c jarq6c commented Dec 9, 2021

External testing found that attempting to compute categorical metrics using contingency tables with zero-valued components resulted in a ZeroDivision error. This behavior was undesirable for downstream users and inconsistent with the behavior of other packages used by hydrotools. This update introduces conversion of contingency table components to numpy.float64 scalars. This has the advantage of inheriting all of the numpy built-in value checking, including raising a RuntimeWarning in the event of division by zero, instead of raising an error.

Additions

  • convert_mapping_values is a new method used to convert dict-like objects to numpy DTypeLike values.

Removals

  • None

Changes

  • Categorical methods all start by calling convert_mapping_values on the input contingency table.

Testing

  1. Explicit tests with null or zero-valued contingency table components
  2. Adding test scenarios for null or zero-valued continuous metrics
  3. convert_mapping_values is specifically tested using a pandas.Series

Notes

  • None

Todos

  • None

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 bug Something isn't working label Dec 9, 2021
@jarq6c jarq6c self-assigned this Dec 9, 2021
@jarq6c
Copy link
Collaborator Author

jarq6c commented Dec 9, 2021

Related to an issue originally reported by @RossWolford-NOAA

Copy link

@christophertubbs christophertubbs left a comment

Choose a reason for hiding this comment

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

Took me a sec to see what the before and after of convert_mapping_values was gonna be, but it seems pretty good to me.

@jarq6c
Copy link
Collaborator Author

jarq6c commented Dec 9, 2021

Took me a sec to see what the before and after of convert_mapping_values was gonna be, but it seems pretty good to me.

Thanks for the review!

@jarq6c jarq6c merged commit 4eb6f87 into NOAA-OWP:main Dec 9, 2021
@jarq6c jarq6c deleted the test-metrics branch December 9, 2021 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants