Add 'simplify' argument to bdh and bds (closes #353)

[eddelbuettel]

This PR aims to follow up on the discussion in #351 about returning bds to pre-3.11 behavior, and implements the suggestion in #353 to add a new argument 'simplify' which, if TRUE, shrinks a one-element list to its sole element.

I added some simple tests. If anybody with the ability to test could do so I would appreciate it. (Remember to set RunRblpapitUnitTests to yes so that the tests are not skipped.)

And please consider the PR a draft to get further discussion going. Comments welcome.

(Plus an explicit tag of @mtkerbeR whom I cannot poke with a formal review request)

[mtkerbeR]

@eddelbuettel Thanks - sure, will have a look at the tests, probably tomorrow.

However, I think the current behaviour of function bdh correspond to simplify=TRUE (which should then probably be the default value to keep current result type) as lists of length 1 are converted to its first elements while list of length > 1 are not:

library(Rblpapi)
packageVersion("Rblpapi")
#> [1] '0.3.12'

blpConnect()
res_1 <- bdh("DAX Index", "PX_LAST", start.date = Sys.Date()-10)
class(res_1)
#> [1] "data.frame"

res_2 <- bdh(c("DAX Index","SPX Index"), "PX_LAST", start.date = Sys.Date()-10)
class(res_2)
#> [1] "list"

[eddelbuettel]

It is entirely possible that I bungled something but as it is also not entirely clear what you tested (CRAN version? Why?) could you possibly

• install from the branch / pr to get version 0.3.12.1
• communicate what change you want in form of a diff or patch relative to that version
• maybe be so kind to wrap "triple-backtick and r" around your code chunks in posts (I'll edit it now)

As for what behaviour we want as default, I am unclear myself. We have a full 2x2 matrix of <bdh, bds> \times <TRUE, FALSE> and I am not sure what behaviour I want. Maybe always simplify == TRUE? Maybe always past behavior (pre 3.11) ?

[mtkerbeR]

Regarding your last questions: I think "always simplify=TRUE" would be the same as "past behaviour (pre 3.11.)".

I wanted to use the code example (thanks for reformatting) to show this for current CRAN version of bdh, namely that its result (type) corresponds to your implementation of simplify=TRUE (supporting my statement/answer above). Sorry for being confusing/unclear here.

[eddelbuettel]

Regarding your last questions: I think "always simplify=TRUE" would be the same as "past behaviour (pre 3.11.)".

That would also simplify the use of simplify=getOption("blpSimplify", TRUE)) because we would get by with just one not two currently committed. Sorry for not remembering correctly, I think you are correct. We used to always simplify. Having an option is nice for the case we discussed of 'sometimes' having more than one argument and hence forcing type-consistent returns.

[eddelbuettel]

@mtkerbeR Thanks again for the earlier comment and check. I pushed an update defaulting to TRUE and bdpSimplify.

[mtkerbeR]

After changing line 30 & 31 in test_bds.R to below (currently the word "info" is missing in line 30 and occures twice in line 31), all unit tests are passing.

expect_error(bds(c("DAX Index", "SPX Index"), c("INDX_MEMBERS", "IVOL_SURFACE")), info = "more than one security and more than one field")
expect_error(bds("DAX Index", c("INDX_MEMBERS", "IVOL_SURFACE")), info = "more than one field")

[eddelbuettel]

Whoops, fixed. My bad for the fat finger there -- tried to fix your misaglined info comment (needed swapping between lines 30 and 31) but messed that up.

As it is just a PR to master and not release critical til we release I may merge this later, but I will maybe wait til end of day for a nod.

[eddelbuettel]

Nobody cried uncle so in it goes. Thanks for the continued help, @mtkerbeR !