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.12% -0.46%
==========================================
Files 280 155 -125
Lines 21012 18629 -2383
==========================================
- Hits 20294 17907 -2387
- Misses 718 722 +4 ☔ 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.
* All MA functions returns a maType-class list
if x is missing - this functionality has always
been in {talib} but without the explicit classes
and dispatches.
With the current changes so far, the S3 serves a rather
crucial role in translating the maType-list back and forth
from C and the charts.
It was mainly implemented to fix ta_VOLUME which would
crash due to the C-side extracting the first argument
of the list that was coerced to a integer vector.
* With this new S3 helper charting and new functions
should become easier - this is what we call positive externality
in economics.
* All vignettes render cleanly locally.
* This vignette was written "just" to to show {tidyverse} could
be used for future tidy-people.
The vignette was written purely with AI, without any consideration
for the actual use-case or necessity.
- Off it goes!
* Some indicators requires custom examples to run like, for example, the ta_MAVP - the new boolean flag takes precedence over any auto-generated example.
* This keeps R CMD check happy!
* The helper files are run before the tests starts altogether. So the data can be properly prepared *once* and passed downstream.
* This is an artifact from X-macro PR where it
seemed like a good idea to add void to the C-signature
+ This commit is mostly to satisfy R CMD check rather than
a proper fix on anything. Most of the zzz.R functions onload and unload
are obsolete as its handled on the C-side anyways.
* Re-introduced rolling minimum, maximum and sum.
+ The naming has been changed to accomodate the more
verbose naming.
* All Math Operators and Statistics Functions are now tagged as Rolling Statistics.
* Some (only one really) functions have an additional passthrough parameter
that does not fit in the OHLC-V schema.
- The specific offender is the MAVP which has a periods argument.
* With this commit any future additions of functions with passthroughs
that are not OHLC-V can be easily integrated into {talib}, without
having to consider a rewrite.
* The MAVP have been regenerated so it accepts the periods parameter. This has introduces a cascade of changes in the downstream files (tests, examples, etc)
* The ROCR were incorrectly excluded during the refactor.
* All functions have a lookback calculation and these should be included by default with their alias and function names.
* This is a work in progress.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
* This commit simplifies the Makefile and exploits R's built-in buildtools as they should be used. R CMD check can be used without R CMD INSTALL, and the checks can be done targeted (it was previously implemented really greedily) - this change speeds-up check-time significantly without sacrificing the reliability. For changes that are going to be pushed to remote or published to CRAN check-cran should be used as it checks for CRAN relevant aspects (Vignettes, README, etc). * The real addition to the Makefile is TA-Lib which is accounted for in all relevant targets. When installed configure automatically uses the system-level TA-Lib which prevents CMake being triggered everytime - this speeds up compilation and checks even further. check-cran forces using the vendored TA-Lib to check whether the configure is configured correctly. But this is already checked via CI/CD, so its really an additional precaution and a local development check in cases where the configure is changed. The speedup in building and checking makes the somewhat redundant vendored install check, even if configure is unchanged, bearable. This commit closes #81 disregarding the proposed changes in the issue as the existing configure, and more knowledge about R's buildchain, already solves all the issues.
* All Maketargets have a .PHONY-target to prevent colliding targets and folder/file names triggered by {pkgdown} locally. This is purely from a "good practice" perspective.
* Added social preview and logo code in .svg and .png for various SoMe posts - branding and all that.
* The CI/CD now builds the favicons before publishing the website to avoid crashing and increasing the number of folders in the repository. * Changed opengraph image to the social preview and added a proper alt-text that describes what it actually shows.
* On the plotly backend, merging indicators onto one panel stacked all last-value labels on the panel's top-right corner. Labels are now collapsed into one evenly spaced, spec-named label, e.g. RSI(10): 39.77 RSI(14): 43.37, matching the ggplot2 backend.
* Each function is now (also) aliased against a camelCase
function. This is a simple operation on the existing snake_case names
which introduces a form of consistency acress R's existing finance ecosystem
- Quantmod uses camelCase, TTR uses a mix of camelCase and PascalCase while PerformaceAnalytics
uses PascalCase entirely.
Using camelCase is proper middle ground between snake_case and PascalCase - and while it is easy
to add PascalCase, there is also a limit to how many different aliases that should/could be sensibly included.
Just because "I can" does not mean "I should." This is mantra with this commit.
If there is demand for it, PascalCase could be introduced.
* README and NEWS have been updated to reflect this addition, with a brief description of the existing TA-Lib alias
which was never really described in the NEWS before now.
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.