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 test_bds.R #352

Merged
merged 2 commits into from
Dec 23, 2021
Merged

Add test_bds.R #352

merged 2 commits into from
Dec 23, 2021

Conversation

mtkerbeR
Copy link
Contributor

As discussed in #351 :

Adds a file to test return type of function bds.
Note that this checks for the behaviour as of 0.3.11, i.e. bds returning a list whose first element is a data.frame

@eddelbuettel
Copy link
Member

Lovely. You even put the old comment around as if it had been a RUnit function ;-) And the header / copyright comment was of course meant as you moment to shine. We can still tweak.

Otherwise good to merge, I guess, and we can sort out if/when to adjust the state of 3.11. Maybe adding boolean flag shortenResult=FALSE can be added that leaves it as a list by default, but if true returns the more compact object?

Copy link
Member

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

LGTM apart from minor niggles over the header comment.

To be confirmed how we best use this.

@mtkerbeR
Copy link
Contributor Author

@eddelbuettel Thanks for your review - if you ever want to change back to RUnit, not much code to change ;-)

From my point of view, having some boolean flag like shortenResult (or simplifySingleton) would be a good solution to switch between both return types (and modify existing code written before resp. since the change depending on the default).

Maybe the same parameter could also be used for function bdh - and if so, it might probably be set to TRUE to keep the current behavior of bdh (and get back the former behaviour of bds)?

On the one hand, default return types of functions bds and bdh would be similar (if the resulting list is of length 1, return the first entry, otherwise return a list - which might be relevant if a future version of bds accepts more than one security).

On the other hand, such a parameter for bdh would allow for something like the code below, without having to take care of the special case of securities being of length 1:


sec_1 <- c("DAX Index")
sec_2 <- c("DAX Index", "SPX Index")

fields <- "PX_Last"

library(Rblpapi)
library(zoo)
blpConnect()

prices_1 <- bdh(sec_1, "PX_LAST", Sys.Date()-10, returnAs = "zoo", simplifySingleton = FALSE) |> do.call(what = "merge")
prices_2 <- bdh(sec_2, "PX_LAST", Sys.Date()-10, returnAs = "zoo", simplifySingleton = FALSE) |> do.call(what = "merge")

@eddelbuettel
Copy link
Member

Yes, exactly. Nice example! The point is to both "limit surprises when scripting" but also be useful when noodling more interactively.

@eddelbuettel
Copy link
Member

Ok, lemme merge this and maybe propose a follow-up PR with the 'simplify' argument (borrowing from sapply) in the next few days. I will need one of you good folks to test with Bbg.

@eddelbuettel eddelbuettel merged commit 5978c76 into Rblp:master Dec 23, 2021
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.

2 participants