joshuaulrich / quantmod

Quantitative Financial Modelling Framework
http://www.quantmod.com/
GNU General Public License v3.0
818 stars 224 forks source link

Improved column matching for OHLCVA #306

Open nocodehummel opened 4 years ago

nocodehummel commented 4 years ago

The functions to match and extract OHLCVA columns return information about OHLCVA data in the input. The implemented logic aims to find the right column also when columns with similar names exist.

This change resolves issues in matching the correct column(s). Known bugs are when the column name has multiple similar matches. E.g. the name is similar to the symbol.

The solution is first attempting an exact column name match using which (this was done with attr). When this fails the fallback is to search for column name(s) ending with the matching term using grep.

This change also removes code duplication that existed between related functions (e.g. has.Op and Op), since each pair related functions attempted to match column names with grep.

As requested test coverage is added with tinytest.

nocodehummel commented 4 years ago

In response to @joshuaulrich comments in #305 . 1) I started by trying to find test case(s) to return a result from the column attribute handling (attr(x, "Op")), but could not verify the functionality with a test case. Do you know a data structure for which this should return a result? What is the expected behaviour?

2) n <- which(colnames(x) %in% c("Op", "Open")) is introduced to replace attr(x, "Op"), plus covering the sample data where column name is "Open". This passed my test cases, where the name matches exactly. Since I could not verify 1) with a test case, I may have jumped to conclusion about what the expected behaviour is.

3) yes, agreed - I started by adding the tests! Therefor 1) and 2). Btw; thanks for pointing out tinytest, started to use it in my own work now.

Looking forward to your feedback on the PR.