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: Forward-fill fx rates past file end. #2644

Merged
merged 1 commit into from
Feb 4, 2020
Merged

Conversation

ssanderson
Copy link
Contributor

@ssanderson ssanderson commented Feb 4, 2020

If an FX rate query requests a date that's greater than the last date in the fx
rate file, forward-fill from the last value in the file rather than raising an
error.

We do this for a few reasons:

  1. We'd like to gracefully handle the possibility of an FX rates file that's
    older than another input file.

  2. Relative to other non-erroring behaviors, forward-filling is the simplest
    thing to implement. Specifically, it's what the implementation prior to this
    change would do naturally if there weren't an explicit check to prevent it.

  3. For an FX rates file containing prices on a 24/5 calendar, some amount of
    forward-filling is required to handle any market with a non-weekday date.

Copy link
Contributor

@peterhbromley peterhbromley left a comment

Choose a reason for hiding this comment

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

LGTM

If an FX rate query requests a date that's greater than the last date in the fx
rate file, forward-fill from the last value in the file rather than raising an
error.

We do this for a few reasons:

1. We'd like to gracefully handle the possibility of an FX rates file that's
   older than another input file.

2. Relative to other non-erroring behaviors, forward-filling is the simplest
   thing to implement. Specifically, it's what the implementation prior to this
   change would do naturally if there weren't an explicit check to prevent it.

3. For an FX rates file containing prices on a 24/5 calendar, some amount of
   forward-filling is required to handle any market with a non-weekday date.
@coveralls
Copy link

coveralls commented Feb 4, 2020

Coverage Status

Coverage increased (+0.002%) to 88.277% when pulling 2546708 on ffill-past-fx-end into 7c654ef on master.

@ssanderson ssanderson merged commit 8195f3a into master Feb 4, 2020
@ssanderson ssanderson deleted the ffill-past-fx-end branch February 4, 2020 23:54
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

3 participants