Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add Indicators.jl functionality with TSFrames #155

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

smishr
Copy link

@smishr smishr commented Mar 3, 2023

Been looking to use methods defined in Indicators.jl with TSFrame objects. Since Indicators.jl defines its methods using simple AbstractArray and Matrix types, it is relatively easy to integrate the two packages.

I tried integrating the runmean and sma (simple moving average) method with TSFrames, and its relatively easy to integrate. It will be great to have those moving average and momentum based operations working out of box with TSFrames.

@smishr smishr changed the title Add Indicators.jl functionality with TSFrames WIP: Add Indicators.jl functionality with TSFrames Mar 3, 2023
@smishr smishr added enhancement New feature or request good first issue Good for newcomers labels Mar 3, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #155 (50c9a5f) into main (30902da) will decrease coverage by 0.45%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #155      +/-   ##
==========================================
- Coverage   91.41%   90.97%   -0.45%     
==========================================
  Files          20       21       +1     
  Lines         431      432       +1     
==========================================
- Hits          394      393       -1     
- Misses         37       39       +2     
Impacted Files Coverage Δ
src/TSFrames.jl 100.00% <ø> (ø)
src/indicators.jl 0.00% <0.00%> (ø)
src/utils.jl 86.36% <0.00%> (-0.21%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@smishr smishr marked this pull request as draft March 3, 2023 09:45
@asinghvi17
Copy link
Member

asinghvi17 commented Mar 10, 2023

Should we make this a package extension and use Requires.jl for Julia < 1.9, to avoid adding another dependency?

@smishr
Copy link
Author

smishr commented Mar 12, 2023

Should we make this a package extension and use Requires.jl for Julia < 1.9, to avoid adding another dependency?

Sure. im not too familiar with how package extensions will work in Julia, but there is blog post here.

Comment on lines 3 to 4
runmean(X::TSFrame, x::Symbol; args...) = TSFrame(Indicators.runmean(X[:,x]; args...))
sma(X::TSFrame, x::Symbol; args...) = TSFrame(Indicators.sma(X[:,x]; args...))
Copy link
Member

@asinghvi17 asinghvi17 Mar 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
runmean(X::TSFrame, x::Symbol; args...) = TSFrame(Indicators.runmean(X[:,x]; args...))
sma(X::TSFrame, x::Symbol; args...) = TSFrame(Indicators.sma(X[:,x]; args...))
Indicators.runmean(X::TSFrame, x::Symbol; kwargs...) = TSFrame(Indicators.runmean(X[:,x]; kwargs...))
Indicators.sma(X::TSFrame, x::Symbol; kwargs...) = TSFrame(Indicators.sma(X[:,x]; kwargs...))

Probably better to be explicit when adding a method like this, things become easier to interpret. Also, conventionally keyword arguments are splatted into kwargs... (not a major thing, but is easier to read).

Also, should this somehow preserve the index of ts?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool

Copy link
Author

@smishr smishr Sep 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you meant by preserving the index of ts?

julia> sp500_2y = TSFrames.subset(sp500, date_from, date_to)
505×6 TSFrame with Date Index
 Index       Open     High     Low      Close    AdjClose  Volume    
 Date        Float64  Float64  Float64  Float64  Float64   Float64   
─────────────────────────────────────────────────────────────────────
 2021-03-01  3842.51  3914.5   3842.51  3901.82   3901.82  5.11482e9
 2021-03-02  3903.64  3906.41  3868.57  3870.29   3870.29  5.53601e9
 2021-03-03  3863.99  3874.47  3818.86  3819.72   3819.72  6.17366e9
 2021-03-04  3818.53  3843.67  3723.34  3768.47   3768.47  7.1954e9
 2021-03-05  3793.58  3851.69  3730.19  3841.94   3841.94  6.85107e9
                                                         
 2023-02-23  4018.6   4028.3   3969.19  4012.32   4012.32  3.95294e9
 2023-02-24  3973.24  3978.25  3943.08  3970.04   3970.04  3.8777e9
 2023-02-27  3992.36  4018.05  3973.55  3982.24   3982.24  3.83695e9
 2023-02-28  3977.19  3997.5   3968.98  3970.15   3970.15  5.0434e9
 2023-03-01  3963.34  3971.73  3939.05  3951.39   3951.39  4.24948e9
                                                     495 rows omitted
...
...

julia> using Indicators

julia> sma(sp500_2y, :AdjClose)
505×1 TSFrame with Int64 Index
 Index  x1      
 Int64  Float64 
────────────────
     1   NaN
     2   NaN
     3   NaN
     4   NaN
     5   NaN
          
   501  4076.32
   502  4065.17
   503  4054.35
   504  4037.64
   505  4019.16
495 rows omitted

Should index of "Date" remain preserved? I think for most Technical Indicators it makes some to preserve.

But there could also be some where the Timestamp is not meaningful. Also there is "burn in" for n number of observations in rolling statistics and trading indicators.

I'll proceed backwards from a TDD paradigm, dev and test and will see works

@smishr smishr linked an issue Mar 15, 2023 that may be closed by this pull request
Project.toml Outdated
@@ -6,6 +6,8 @@ version = "0.2.0"
[deps]
DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
Indicators = "70c4c096-89a6-5ec6-8236-da8aa3bd86fd"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in [weakdeps]


# Methods for porting Indicators.jl functions to TSFrame objects from TSFrames.jl package
# See their respective documentation on Indicators.jl
Indicators.runmean(X::TSFrame, x::Symbol; kwargs...) = TSFrame(Indicators.runmean(X[:, x]; kwargs...), X[:, :Index])
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preserving Index. Is there better way to integrate to TSFrame? More optimised way to avoid extra Construction operations?

Indicators.kama(X::TSFrame, x::Symbol; kwargs...) = TSFrame(Indicators.kama(X[:, x]; kwargs...), X[:, :Index])
Indicators.alma(X::TSFrame, x::Symbol; kwargs...) = TSFrame(Indicators.alma(X[:, x]; kwargs...), X[:, :Index])
Indicators.zlema(X::TSFrame, x::Symbol; kwargs...) = TSFrame(Indicators.zlema(X[:, x]; kwargs...), X[:, :Index])
# VWMA is defined on Matrix, not Array
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still thinking how to adapt methods that work on AbstractMatrix, not Array

@@ -1,6 +1,6 @@
module TSFrames

using DataFrames, Dates, ShiftedArrays, RecipesBase, RollingFunctions, Tables
using DataFrames, Dates, ShiftedArrays, RecipesBase, RollingFunctions, Tables # , Indicators
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To remove comment

@@ -97,5 +99,6 @@ include("to_period.jl")
include("vcat.jl")
include("broadcasting.jl")
include("tables.jl")
# include("indicators.jl")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rm comments

date_from = Date(2021, 03, 1)
date_to = Date(2023, 03, 1)
sp500_2y = TSFrames.subset(sp500, date_from, date_to)
@test sma(sp500_2y, :AdjClose) |> typeof == TSFrame
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make tests more diverse and meaningful, by using kwargs from upstream method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integration and documentation with Indicators.jl
3 participants