returnAs=... support for bdh (closes #206) (rebased)

[eddelbuettel]

This picks up the nice work by @mtkerbeR in #334 but places it cleanly off the head of our repo. It also add a ChangeLog entry and updated the Rd page.

Original motivation from #334 follows.

A first proposal of minor changes to code for function bdh() to return different types of objects - analogously to getTicks()

As this is one of my first contributions in R, please let me know about feedback/weaknesses of the proposed solutions, which I would be happy to take care of:

Two points worth mentioning:

1. a new Option bdhType is introduced, to allow for backward-compatibility. Alternative would be to use blpType here as in getBars, however, if this value is set to a format which is not supported in bdh (like matrix, which is feasible in getBars) existing code might break. This could be catched by extending possible values in match.arg(returnAs, c("data.frame", "fts", "xts", "zoo", "data.table")) by getOption("blpType"), which would later on leads to a data.frame via the fallback option in switch if format is not supported.

2. If some columns are non-numeric, types inheriting from zoo will convert numbers to characters for all columns (due to inheritance from matrix)

The code could be (unit)tested as follows - adapting existing test cases for bdh to the case of different values for parameter returnAs:

sec <- "TY1 Comdty"      
fields <- c("PX_LAST","OPEN_INT","FUT_CUR_GEN_TICKER") # probably also worth testing with fields of length 1, due to drop = FALSE
retAs <- c("data.frame", "fts", "xts", "zoo", "data.table")

res <- lapply(type, function(x) bdh(sec, fields, Sys.Date()-10, returnAs=x))

expect_true(all(sapply(seq_along(retAs), function(x) inherits(res[[x]], retAs[x]))), info = "checking return type")
expect_true(all(sapply(res, nrow) >= 5), info = "check return of five rows")
expect_true(all(sapply(res[sapply(res, function(x) !inherits(x, "zoo"))], ncol)==length(fields)+1), info = "check return of four columns for non-time series")
expect_true(all(sapply(res[sapply(res, function(x)  inherits(x, "zoo"))], ncol)==length(fields)),   info = "check return of four columns for time series")
expect_true(all(sapply(res, function(x) tail(colnames(x), length(fields)) == fields)), info = "check column names")

[eddelbuettel]

Hi @johnlaing if you have a moment today or tomorrow, could you glance at this? Looks like another fine contribution by @mtkerbeR who, unfortunately, started from a very stale repo so I cherry-picked his two commits over and completed the PR.

[johnlaing]

See this, will check as soon as I'm able. Having some build difficulties myself at the moment.

[johnlaing]

We should add tests for this feature. @mtkerbeR proposes them in the text of the PR, but I don't see them in the code. I can add.

[johnlaing]

@eddelbuettel I don't think I can request your review since it's your PR, but if you could have a quick look please. All tests pass with a BB connection.

[mtkerbeR]

Sorry for being so late to respond.

@johnlaing Thank you very much for including the tests! Just a minor point: I think PX_CLOSE should probably be PX_LAST (as the former does not seems to exists per BBG FLDS and leads to a column of NAs) - tests pass nevertheless (as expected).

[eddelbuettel]

John or I can probably slab a quick clean-up commit on that. Was also thinking rolling the verison to 0.3.11.1 may be clean.

To be precise here, can you show a short diff of which file and where? I presume you talk about the test file?

[mtkerbeR]

As I'm currently not on a computer running git, please accept diff as plain text:

Testfile inst/tinytest/test_bdh.R, line 44 resp. 48 contains non-existing field "PX_CLOSE":

res <- bdh("TY1 Comdty",c("PX_OPEN", "PX_HIGH", "PX_LOW", "PX_CLOSE"),Sys.Date()-10,returnAs=retAs) expect_true(all(c("PX_OPEN", "PX_HIGH", "PX_LOW", "PX_CLOSE") %in% colnames(res)), info = paste("check column names -", retAs))

which should be replaced by "PX_LAST":

res <- bdh("TY1 Comdty",c("PX_OPEN", "PX_HIGH", "PX_LOW", "PX_LAST"),Sys.Date()-10,returnAs=retAs) expect_true(all(c("PX_OPEN", "PX_HIGH", "PX_LOW", "PX_LAST") %in% colnames(res)), info = paste("check column names -", retAs))

[eddelbuettel]

Thank you that's perfect as it states two lines. I'll that in in a moment.

[eddelbuettel]

Went "formal" as we've been so well behaved lately so it is now a fresh PR: https://github.com/Rblp/Rblpapi/pull/336