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

MAINT: Clarify edge case handling for FX Rate Readers. #2640

Merged
merged 6 commits into from
Jan 31, 2020

Conversation

ssanderson
Copy link
Contributor

  • When reading before the start of data, return NaN. We do this because it's
    hard to reliably apply a lower bound to the queried dates in core-loader
    style pipeline loaders.

  • When reading an unknown base currency, return NaN. We might get data from
    third parties with unknown currencies. Doing so should not be an error.

  • When reading after the end of data, emit an error rather than forward-filling
    forever. We may want to revisit this in the future.

peterhbromley and others added 2 commits January 23, 2020 10:51
- When reading before the start of data, return NaN. We do this because it's
  hard to reliably apply a lower bound to the queried dates in core-loader
  style pipeline loaders.

- When reading an unknown base currency, return NaN. We might get data from
  third parties with unknown currencies. Doing so should not be an error.

- When reading after the end of data, emit an error rather than forward-filling
  forever. We may want to revisit this in the future.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.9%) to 87.371% when pulling a7e0898 on fx-changes-for-estimates into 8443934 on master.

@coveralls
Copy link

coveralls commented Jan 30, 2020

Coverage Status

Coverage decreased (-0.002%) to 88.287% when pulling baeac17 on fx-changes-for-estimates into 8443934 on master.

Scott Sanderson added 2 commits January 30, 2020 10:59
This handles None more gracefully than coercing to numpy's string type, and it
also ensures that we satisfy the documented input types for get_rates.
We should always return NaN on a request to load None as an FX rate.
@ssanderson ssanderson merged commit 008d719 into master Jan 31, 2020
@ssanderson ssanderson deleted the fx-changes-for-estimates branch January 31, 2020 19:02
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.

None yet

2 participants