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

bdh function: include.non.trading.days not working in conjunction with other options. #167

Open
ghost opened this issue Apr 20, 2016 · 1 comment

Comments

@ghost
Copy link

ghost commented Apr 20, 2016

This issue pertains to attempting to pull historic data (bdh function) for all real days, and setting the way NA values are handled. This filling does work, when all options are set within the options argument, but does not work when some options are set in options and include.non.trading.days is used. Below is a simple example:

library(Rblpapi)


blpConnect()


# Print 30 real days of data, without backfilling missing values:
bdh("VIX Index", "PX_LAST", start.date = Sys.Date() - 30, include.non.trading.days = TRUE)



# Now, try to use the option to fill NA's with the previous value:
myOptions <- structure(c("PREVIOUS_VALUE"), names = c("nonTradingDayFillMethod"))

# You'll notice, all the NA's remain:
bdh("VIX Index", "PX_LAST", start.date = Sys.Date() - 30, include.non.trading.days = TRUE, options = myOptions)



# Instead of specifying 'include.non.trading.days' in the bdh function, I'll move this directly to the options:
myOptions <- structure(c("PREVIOUS_VALUE", "ALL_CALENDAR_DAYS"), names = c("nonTradingDayFillMethod", "nonTradingDayFillOption"))
# Then, I remove the 'include.non.trading.days' specification from the function, and it works as expected:
bdh("VIX Index", "PX_LAST", start.date = Sys.Date() - 30, options = myOptions)

This most likely ought to be low priority, since there is a fairly straight-forward work-around. Rather, this is more FYI.

@eddelbuettel
Copy link
Member

Nice work. I will try to replicate.

I think this is a leftover from a 'version 1' of the interface, and I suggest to

  • document the preferred usage (pretty much copying what you contributed)
  • flag the old include.non.trading.days as deprecated
  • treat it with a warning for a suitably long period (one year?)
  • treat it as an error for a suitably long period (one year?)
  • remove it

Thoughts @armstrtw @johnlaing ?

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

No branches or pull requests

1 participant