Conversation
* This commit introduces support for cached builds across installation runs for each pushed update. It supports ccache, sccache and distcc. At this stage its only tested for remote installs, but it *should* cover all edge-cases either way as the compiled library is otherwise the same. * This commit closes #57 by implementing cached builds across installation runs.
* The README has been streamlined and now positions the {talib} relative to the upstream, and (implicitly) relative to the wider R ecosystem.
* The usage examples have been equally weighted between charting and indicators in general, with a generally higher emphasis on the candlestick pattern recognition which is the primary differentiator against {TTR}
* normalize one-column numeric indicator outputs back to double vectors * update the generated numeric templates so future wrappers keep the same behavior * tighten generated numeric tests to reject one-column matrix results while preserving multi-column matrix outputs Fixes #61 --------- Signed-off-by: nvphungdev <283886185+nvphungdev@users.noreply.github.com> Co-authored-by: nvphungdev <283886185+nvphungdev@users.noreply.github.com>
* The benchmarks now covers comparisions against
{TTR} across multiple indicators and presents them
in the README accordingly.
* The benchmarks now compares the .Call against the
dispatched methods more thoroughly than before.
The goal of the benchmarks is primarily micro-optimizations
on the R side.
* The README now includes a subtle reference to the benchmark where the claim of speed is made. * Added linebreak after dots (this is good standard)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## CRAN #65 +/- ##
==========================================
- Coverage 96.58% 96.01% -0.57%
==========================================
Files 280 162 -118
Lines 21012 21685 +673
==========================================
+ Hits 20294 20821 +527
- Misses 718 864 +146 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
* See issue #67 where remote installs fails on MacOS due to missing C header files. The error is related to gettext (source: https://stackoverflow.com/questions/11370684/what-is-libintl-h-and-where-can-i-get-it) The issue have not been addressed in the repository before now as it was assumed to be related to the workflow itself - the failed run coincided with the release of R4.6.0 which turned out to be a false-positive as it still an existing issue. * Since the issue is most likely an upstream issue, the R-devel runs have been removed for all OSes and replaced with oldrel-2. Backwards compatibility is more imporant than experimental releases - if you decide to throw a development version of R against a package that deals with (potentially) expensive implementations, you have bigger things to worry about.
* This PR introduces a lookback function that wraps indicator_lookack that returns <NA> for invalid indicator and data pairs, and always minimum 1 (Can't calculate any indicators without data). The implementation differs from upstream in the way that upstream returns -1 for invalid pairs, and 0 for indicators that in reality does not need a lookback periods (C is 0-indexed). * The implementation required significant backend changes that has no effect on the interface. One such example is using `dim(x) <- NULL` for 1D-arrays instead of `as.double()`; this introduces the following changes: (1) unit-testing has been made more flexible so it tests for `is.integer()` simultaneously as the new approach preserves the underlying storage mode. (2) parity-tests have, for lookback-calculation(s), been adapted to the new calculation of lookbacks. See PR #71 for more details.
* The indicators on C-side were generated using two separate templates for candlestick and non-candlestick. These have been consolidated to ease future changes from upstream and possible interface changes and additions. * The lookback-functions on C-side now ignores ununused arguments to avoid compiler-warnings. The arguments have been kept to streamline function signatures on the R-side.
* Instead of relying on nested if-else statments, the if-else statements are heavily dependent on conditionals to be met.
* This is common knowledge: '<-' is for assignments its unclear when '=' was used in the code. But let this commit be testament to the commitment of correct use of assignment operators. - All hail the assignment operator.
* Without this, the documentation entry does not render.
It appears that {roxygen2} looks for the top-level entry as
the title, unless strictly typed.
* The example in lookback() were breaking because
there was an additional non-existing argument passed
into the downstream function.
* The (possible) fix is to let lookback() be strictly
typed, for the lack of better words, such that it fails
if the lookback signature doesn't match the downstream function
entirely.
This is probably not the best idea, but the lookback function is
a function for 'developers' that seeks to wrap {talib} and it is therefore
expected that the developer is familiar with the function(s).
Even for non-developer use-cases this implementation has its justification
in the sense that if there is a need to pre-calculate the lookback period
it must be due to critical infrastructure implementations, which in return, again,
requires familiarity with the function(s).
Bumps [src/ta-lib](https://github.com/TA-Lib/ta-lib) from `c042ea3` to `bffd837`. - [Release notes](https://github.com/TA-Lib/ta-lib/releases) - [Commits](TA-Lib/ta-lib@c042ea3...bffd837) --- updated-dependencies: - dependency-name: src/ta-lib dependency-version: bffd83732f0ab022a272ae6cd79d1ba6b32cc2ce dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [src/ta-lib](https://github.com/TA-Lib/ta-lib) from `bffd837` to `2247d59`. - [Release notes](https://github.com/TA-Lib/ta-lib/releases) - [Commits](TA-Lib/ta-lib@bffd837...2247d59) --- updated-dependencies: - dependency-name: src/ta-lib dependency-version: 2247d599bddf37ed37e3a709371517e46efc66f6 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
* The code-generator currently only works for the TA-Lib (C-side) indicators and uses the TA-Lib functions list to identify all indicators with their respective signatures wrapped in X-Macros. The goal is to be independent from BASH and use an algorithmic approach so input arguments that has default values are not silently dropped. Furthermore it is expected that ALL TA-Lib indicators will be generated using X-Macros.
* The X-Macros will be using TA_* prefixes for readable preprocessing. So DBL --> TA_DOUBLE for doubles.
* The codegen/ now also parses the TA_<indicator>_Lookback signatures which is the optional inputs for all the functions. * The original C_MACRO has been renamed to BATCH to distinguish between streaming (Upcoming) and batch API for later TA-Lib versions.
* The identifiers follows a simple if-else statement for identifying candlesticks based on the indicator name 'CDL'. With this implementation it is now possible to implement candlestick-specific logic on the C preprocessing side without having to rewrite the logic downstream.
* All lookback related functions have been removed to accomodate the new codegen that uses the exact signature of the TA_<indicato>_lookback funtions.
## 📚 What? This PR is a rewrite of the source code that binds TA-Lib to R. The overall goal is to reduce the amount of code, and simplify the C-wrappers. It has been implemented incrementally, so much of the original functionality as been deleted but is to be reintroduced in a similar fashion. There has been no consideration for backwards compatibility which in this case means that a part of the R logic has to be re-written (This does not affect the signatures at all). All *_lookback functions, for example, now only accepts the relevant arguments in `.Call()` which breaks with the current R side logic. (This should be possible by only rewriting the templates, so the PR is mainly focused on the C-side)
* fmt target now also runs cargo fmt on the crate * gen-code target now includes cargo run * Deleted old script runners
* This commit mainly brings a new backend to the codegeneration on the R side - the old generation were handled by a mix of R and BASH which were harder to maintain than first expected. shQuote() or was it paste()? And how does BASH recieve these arguments? These were questions and debugging the code generation were riddled with. Using Rust unifies the headache in a uniform way, which seems to be easier maintainable.
* The code generation is mainly handled by mining the .xml file that TA-Lib constructs without any modifications to the input arguments. This means two things: (1) breaking changes in R signatures and (2) bug-fixes where MATypes were mixed with multiple indicators (See ta_APO.R) where the MAType's role were ambigious. {talib} is still in pre 1.0.0 and the download counts are so low that there should not be any breakage in dependencies downstream.
* The code generation on the R side is (at this stage) a brute attempt which means that some of the exported functions are either broken or incorrectly named. These will be fixed in the subsequent commits to avoid too many changes in a single commit. This squash commit contains 24 (ish) commits, which makes the development flow non-transparent, which is also why the bugs are kept as-is at this stage.
* The optional documentation is mined fromn the .xml file and manipulated so it emits proper R documentation for each documented parameter.
* This change reflects the new signature the exported functions follow.
Bumps [src/ta-lib](https://github.com/TA-Lib/ta-lib) from `f51cba1` to `a8272d0`. - [Release notes](https://github.com/TA-Lib/ta-lib/releases) - [Commits](TA-Lib/ta-lib@f51cba1...a8272d0) --- updated-dependencies: - dependency-name: src/ta-lib dependency-version: a8272d0c2a90c6974dad2c2e2358c0bd4127d57f dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
* The NAMESPACE were missing the tradingVolume()-alias, its unclear how this were missed in the process - but it does highlight that that the unit-testing is lacking som serious depth for handwritten functions.
* With the PR #83 TA-Lib have introduced new indicators which are implemented in this commit. The Implementation is quick and dirty (ie. it works on the indicator level, but needs polishing on the charting level)
* PR #83 introduced, beyond new indicators, changes to the generated documentation and default values which are reflected in this commit. These changes have not yet been documented in the NEWS.md (I will do that soon (TM)). The first and foremost change which triggered the change in the README is the change in the default value of the bollingerBands timePeriod from 5 to 20. APO and PPO now uses EMA as default (was SMA prior to this commit). * There are also minor and insignificant changes to the documentation itself which is automatically generated which can be seen in the diff.
* The data is extracted using {quantmod} and is stored
as an <xts>-object that is to be used for examples, vignettes
and tests.
* The added class will ease the downstream coding process when adding <xts>-methods to the indicators.
* The index refers to the time-index of the object passed into the indicator functions. By default, for <matrix> and <data.frame>, it (was) is assumed that these are given in the rownames() of the passed OHLC-V object. For an <xts>-object the time-index is given as the index attribute - that is just how <xts> works which necessitates this change. * With this introduction <xts>, <data.frame> and <matrix> objects can be handled in the default method which reduces the number of lines changed for the introduction of the <xts>-methods. Any changes and bugs can potentially be handled in the dispatching functions like, for example, series.xts() to avoid a mass change in files for a simple bug.
* The signature of the series function is now more "straightforward," in the sense that there are no dispatch switching conditional on how the arguments are passed. This was a change that was long due, as it was a rather confusing construction. The series() is now a more proper S3 function that has a method for each of the relevant data classes. * A new (classed) method has been introduced to accomodate the <xts> methods in the indicator functions. The method adds an explicit class to the <xts> method such that the <xts> methods can implemented without further changes to the default methods on the indicator side. There have been introduced a wide variety of functions in utils.R to accomodate this change. * Unit-tests have been rewritten so it fits the new series() structure, which has the same underlying logic but a changed input arguments.
* <xts> methods have been added to all non-rolling indicators (these are considered utility functions) which follows the same underlying logic as the <data.frame> and <matrix> methods; ie. it dispatches to the default method. - The logic are handled by series() and utils.R via S3 classes * The most "important" change here is that the indicators moves from using rownames to index in the argument naming. This is to introduce some sort of consistency across the methods (<xts> does not have any rownmes). Under the hood <data.frame> and <matrix> methods still sets the rowname attribute while <xts> sets the index attribute of the resulting indicator.
* series.xts() now resolves each formula variable to exactly one column
- exact name matches first, then quantmod-style suffix matches
(close -> TICKER.Close), both case-insensitive and regex-free - and
returns the columns in formula order, so the C layer receives
high/low/close/... in the positions the indicator expects regardless
of the physical column layout of the <xts>.
* This fixes silently wrong values for multi-input indicators on
reordered or substring-colliding column layouts, honors explicit
'cols =' selections exactly, errors on missing or ambiguous columns
instead of computing on arbitrary ones, and accepts lowercase
open/high/low/close names.
* Unused model.frame-arguments ('subset',
...) now warn instead of being silently dropped. Chart data overrides
with quantmod-named <xts> (indicator(RSI, data = x)) resolve through
the same path and now work.
* Every .xts method now calls assert_xts(): the {xts} namespace is
loaded on demand via requireNamespace(), with a clear error when the
package is not installed. library(talib); RSI(GOOGL) works in vanilla
sessions - and in zoo-only sessions - instead of dying with
'subscript out of bounds'; the GOOGL man-page example runs under
R CMD check again.
* set_index() gained a default method with a clear message, replacing
the bare 'no applicable method' failure on malformed <xts>-classed
input.
* trading_volume() received the xts rollout the generated wrappers
already had: index()/set_index() plumbing, a trading_volume.xts
method, and the <ta_object> class stamp in src/volume.c that every
generated C wrapper attaches via wrapper.h - VOLUME(xts) returns a
proper <xts> carrying the input's index and the actual volume column.
trading_volume.matrix now wraps in as.matrix() like the template, so
matrix output stays a plain <matrix>.
* Regenerated wrappers via make codegen; the only generated diff is
the assert_xts() guard in each of the 129 .xts methods.
* All seven rolling generics (rolling_maximum, rolling_minimum,
rolling_sum, rolling_standard_deviation, rolling_variance,
rolling_beta, rolling_correlation) gained an .xts method: a
single-column <xts> in, an <xts> out carrying the input's index and
the statistic's column name (MAX, BETA, ...), with values identical
to the vector path. For the bivariate pair, 'y' may be a vector or a
single-column <xts>; pairing is positional.
* Multi-column input is now rejected with a clear error ('Expected
'x' to be univariate.') instead of being silently flattened
column-major into one concatenated series - the guard is generated
per series argument into .default and .xts, covering xts and matrix
input alike. This matches TTR::runMax's univariate contract, and
runMax/MAX now agree exactly on identical input.
* Vector returns from the rolling family drop the stale
c('ta_object','matrix','array') class alongside the dim-strip - a
dimensionless vector no longer claims inherits(., 'matrix') - while
keeping the lookback attribute.
* Wiring: ${SERIES_GUARD} placeholder in render.rs,
rolling_template.R .xts block, generated xts value-parity and
multivariate-error test blocks in testthat.rs (guarded by
skip_if_not_installed('xts')), and the rolling man-roxygen templates
now document the xts contract. Also corrected the template's
fresh-render call site (${C_NUMERIC},,, -> ${C_NUMERIC},), a latent
syntax bug masked by splice preservation.
* The .numeric methods strip the internal class unconditionally:
single-output results are plain double vectors (dim and class
dropped), multi-output results are plain matrices carrying the
implicit c('matrix','array') class - the C-stamped
c('ta_object','matrix','array') tag no longer leaks out of the
vector fast path. Column names and the lookback attribute are kept,
inherits(x, 'matrix') finally agrees with is.matrix(x), and base
as.matrix()/as.data.frame() coerce the results correctly.
* Unknown arguments landing in '...' on the numeric path now warn
("'...' is passed but is unused for vectors.") instead of being
silently swallowed - the only signal a case-typo'd parameter name
(timeperiod = 10) will ever produce. Passing 'cols' keeps its
existing warn-and-compute behavior.
* Generated numeric tests lock the new shapes:
expect_false(inherits(x, 'matrix')) for single-output,
expect_identical(class(x), c('matrix', 'array')) for multi-output.
* tests/testthat/helper-library.R attaches {xts} only when it is
installed, and every generated <xts> test block carries
skip_if_not_installed('xts') - the suite no longer hard-depends on a
Suggests package (verified against a library with xts/zoo hidden:
everything loads, the xts blocks skip cleanly, zero failures).
* Every generated test file gained a 'Value parity with <data.frame>
(<xts>)' block: the <xts> result must equal the lowercase
<data.frame> reference numerically, both on the GOOGL fixture and on
an alphabetically sorted copy that places 'Adjusted' before the OHLCV
columns - locking the column-resolution contract (formula order,
suffix matching, no substring collisions) for all 129 indicators.
* test-series.R gained unit tests for the <xts> resolution layer
itself: formula-order selection, quantmod-style suffix matches,
lowercase names, explicit 'cols =' order, and the error/warning
contracts (no match, ambiguity, short formula, unused 'subset');
plus a vanilla-session subprocess regression (skip_on_cran) proving
RSI(GOOGL) works without {xts} attached.
* series() gained a default method that defers any classed input to as.data.frame() before column resolution - a multivariate zoo (or any data.frame-coercible object) now computes on the coerced frame with the index as rownames, instead of dying with the internal "no applicable method for 'series'" error. Uncoercible inputs fail with base R's own coercion message naming the class. * The indicator documentation now states the actual per-class contract: xts in, xts out alongside data.frame/matrix; coercible inputs computed on the coerced data.frame; the xts column resolution (exact matches first, then quantmod-style suffix matches, consumed in formula order) documented under 'cols'; and '...' marked unused for xts input. * index_data_frame(), index_matrix() and index_xts() in src/names.c now return SEXP (R_NilValue) as Writing R Extensions requires for .Call routines - previously void, an undefined-behavior read of the return register. A registration sweep confirms no other void .Call routines remain.
* The chart dispatch no longer calls ...names() (R >= 4.1): the argument names are already captured by the substitute() call one line above, so names(dots_quoted) carries the identical information. * nullfile() (R >= 4.0) is replaced by an internal .nullfile() in the small base-R replacements section of utils.R, returning the platform null device; the three render-path call sites in chart_indicator.R now use it. Verified against the single-panel print, the multi-panel grob alignment and the print-guard paths. * The pattern-marker data.frames in chart_pattern.R set stringsAsFactors = FALSE explicitly - the default flipped in R 4.0, and the declared floor predates the flip. **NOTE:** This was captured during an adversial review by Fable.
* trading_volume() on a vector no longer errors on the default maType: the numeric splice converts the specifications like .default does.
* 'subset' is evaluated exactly once, by model.frame(), identically on every route (direct data.frame call, wrapper-on-chart, indicator()): data-context expressions like subset = close > 60000 now work everywhere, columns of the data win over caller objects, and the caller's frame is the fallback. The internal re-evaluations that crashed data-context expressions - and evaluated side-effecting expressions up to three times - are gone; the subset attribute now records the row positions model.frame() actually kept. * Arguments on the chart path resolve in the caller's frame: data =/subset = referencing function-local objects work from inside function bodies. indicator() keeps 'subset' quoted and evaluates the wrapper call in the caller's frame, so all routes share one semantic. * The ggplot2 backend aligns indicator positions by axis label, mirroring plotly - indicators computed with subset = are drawn at the subset's own candles instead of positions 1..n. * indicator(FUN, data = y) no longer silently stamps the chart's dates onto a mismatched override: a length mismatch warns and the trace is labelled with the override's own rownames. * chart() normalizes quantmod-style OHLCV names (TICKER.Close, Close) to the lowercase names the backends consume - exact matches first, then dot-suffix matches, mirroring series.xts() - and fails fast with the package's own missing-columns message instead of an opaque backend error at render time. Standalone indicator() with <xts> data keeps its time axis.
* Added <xts>-method mention, and removed the minimal dependency argument - even though xts and zoo are suggested it is still a departure from the minimal dependency framework. * Replaced BTC with GOOGL to demonstrate <xts> compatibility with class in and class out condtion.
* The .md file were not rendered or updated. This is what happens if you commit and rebase before bedtime.
* Moving averages now use foo(x, <series>, <options>, cols, na.bridge, ...). VWMA's volume is a formal of the generic with the ~close + volume column as fallback, fixing the 3-vs-4 parameter warning in checkFF(). Breaking: positional 'cols' as second argument in MA calls.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This PR is a WIP.