Refactor code - #5
Merged
Merged
Conversation
* The ta_INDICATOR.R are now being generated via indicator_template.R and generate_indicator.sh There is no "real" reason for this other than it eases the development flow when there are new indicator-wise additions or removals that needs to be done. For example adding assert_plotly() functions can be done in indicator_template.R and then all scripts will adapt downstream.
* The core implementation of the acceleration bands were bugged: the 'lower' and 'upper' bands were reversed * The R implementation have been reworked and now follows the generated template.
* New range of assert and asser_* functions that
produces informative error message which blames
the parent caller.
It might be more optimal to blame the top caller from
the call stack.
* Deleted outcommented code, and moved some functions to the
helper.R file.
- The distinction between helper.R and utils.R has always been really
fluffy, so there is not "real" decision rule on which functions go
to which file.
* The assertions of formula (cols) length in indicator
functions are moved to series. This has the following benefit:
- All indicators are passing default formulas to series() anyways
so the assertion only have to be written once.
* The core implementations of the functions have been "sligthly" reworked. Both functions now return a named matrix. * The R implementations have been reworked and now follows the generated template.
* When indicator() is called on indicators that is applied
on the main chart like, for example, SMA(), acceleration_bands(), etc.
it would fail if chart() was not called.
While it is somewhat irrelevant to chart an indicator that is read together
with the price action, it does support the overall idea of {talib}: flexibility.
* The function is partially autogenerated and needs further documentation. But this will come later (TM)
* e7a60a7 triggers a full revision of the family-level indicators. - This is really something that should be handled by Github Actions Note to self: Implement it.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #5 +/- ##
==========================================
+ Coverage 88.78% 91.43% +2.64%
==========================================
Files 198 232 +34
Lines 10196 10899 +703
==========================================
+ Hits 9053 9965 +912
+ Misses 1143 934 -209 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* The generate_indicator.sh is now label-aware which enables changes into the template without having to re-write everything downstream. * The indicator_template.R have been adapted to the new shell script
* The dates were intended for tracking, but every addition of functions will trigger a full range of changes which is undesirable from a maintanance point-of-view.
* All functions now returns a matrix where it was originally returning a vector.
* The momentum indicators are regenerated with the new generator. Its currently bugged the F out. - Buts its a WIP.
* The core implementation had stack imbalances due to a mismatch between PROTECT/UNPROTECT - Reformatted the code while already at it. * The R implementation were referring to non-existing variables from the old implementation
* Lower/Upper values were not provided. The functions needs a fresh approach either way, so now we try it like this. * The returned values from core were renamed to ULTOSC from ultimate_oscillator - I acutely need a proper naming convention!
* The STOCHRSI now handles the offsetting on the C-side instead of using na_pad. This approach is more efficient than using 'c()' or 'cbind()' for really large matrices or data.frames. Missing values from RSI is extractaed manually instead of using is.na(), the number of <NA> is deterministic upto N + N_RSI. However, this MIGHT mess stuff op.
* NOTE: The APO() function needs proper work. This will
be done at a separate branch.
serkor1
force-pushed
the
refactor_code
branch
2 times, most recently
from
October 22, 2025 17:57
88c4f9c to
a312b96
Compare
* ADX, ADXR, BOP, MOM and WILLR have been added to the pool of Momentum Indicators * Unit-tests have been updated accordingly.
* These arguments were a part of the original implementation before
moving to autogenerated scripts
- Most of the autogenerated functions are missing their args in fact.
Its probably best to do it in a separate PR.
* Rendered documentation.
serkor1
force-pushed
the
refactor_code
branch
from
October 22, 2025 17:59
a312b96 to
49109a8
Compare
* The naming convention follows TA-Lib closely with this change.
* Unit-tests updated accordingly
* I will skip the scaled versions of ROC and ROCR. Its such a trivial addition that it makes no sense to add. To achieve the same thing in C, you'd need to write a full for loop, or a branched loop. So it makes sense there, not here. The real challenge is whether my OCD can handle it, lol.
* This really needs to stop.
* The MA now no longer uses the maType-spec function away it goes. And so does na_pad, its no longer needed since e7bdcf2
* The MA functions now returns a named matrix from C directly and is now behaves similar to the remaining functions * The function naming follows the rest of library snake_case + alias that is consistent with TA-Lib * Updated unit-tests and documentation accordingly
* README were using an old and experimental API of the MA-functions. * Repeated as.integer conversions fixed.
serkor1
force-pushed
the
refactor_code
branch
from
October 23, 2025 19:22
b2007a9 to
3338f2f
Compare
* All unit-tests now tests for equal lengths for <data.frame> and <matrix>
- The main goal is to have .numeric methods, and these are messing with the .matrix methods.
But that is for later.
* The template now accounts for <matrix> and <numeric> methods more robustly. Using NextMethod() is just too shaky here, or I am just really bad ¯\_(ツ)_/¯
* See 58016a9 if necessary for more details. A proper MA method should apply to a <vector>, without that aspect its just too complicated.
* Templates are moved to /tools/templates
* The rownames should be extracted *after* the call to series to prevent mismatches between rowname length post subsetting (if passed)
* The candlestick patters are generated by candlestick_template.R and (somewhat) follows the remaining structure. The core implementation *should* return a matrix directly but at this stage it is not worth the time to manually code it as all of the core functions just returns a vector, so its easier to just return a vector and then convert it to a matrix. * The candlestick patterns needs more work as there are candlestick relevant options that are passed directly to the C-side, so therefore the above point and the relevant options will be addressed at a different branch. * All tests passes, but its highly unlikely that checks will pass as 'eps' has been removed in some cases.
* The template used for 770bc3a
* Follow-up from a4977d4
* The generate_indicator.sh now accepts a template parameter
(for obvious reasons)
* Added callers for each family of functions
(Cumbersome work)
* This has downstream effects. See next commit.
* From 40f72a5 the bollinger_bands have been slightly changed in color, this affects the README * The SAR and SAREXT have been renamed so all all related .Rd files are also affected.
* Some of the functions have been renamed. - You know the drill * New generator for cycle indicators
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
1 participant
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.
📚 What?
This PR comes with some significant changes to the back and front end of the library. Mainly:
indicator()for indicators that applies to candles can now be charted independently of whetherchart()has been called.🔖 Important
All
.Rfunctions are now auto-generated and "many" of the early C-functions still use un-abstracted functions to generate matrices. This goes for, especially, the Hilbert Transform functions.