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

MFI Indicator #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

MFI Indicator #30

wants to merge 2 commits into from

Conversation

darthur11
Copy link

I'm not expert in Julia, it's my first attempt, so if you have any suggestions I'm glad to make edits. Also, I plan to implement OBV too.



"""
MFI (Money Flow Index) gives almost the same results as tradingview.com
Copy link
Owner

Choose a reason for hiding this comment

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

Documentation comments should follow the same convention as other functions (function signature in triple-backticks followed by indicator name/interpretation two lines below).

"""
MFI (Money Flow Index) gives almost the same results as tradingview.com
"""
function MFI_logic(x::Array{T}; n::Int64=14, args...)::Array{T} where {T<:Real}
Copy link
Owner

Choose a reason for hiding this comment

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

Function naming convention is all lowercase, e.g. mfi.

end

##### X::TS should contain traditional OHLC+Volume too
function MFI(X::TS, flds::Vector{Symbol}; args...)
Copy link
Owner

Choose a reason for hiding this comment

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

Same comment about naming convention. The MFI method that takes a TS type as input should go in src/temporal.jl and should have the same function name as the base Array type method (mfi). Julia will automatically know which method to apply based on the input types :)

Copy link
Owner

@dysonance dysonance left a comment

Choose a reason for hiding this comment

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

@darthur11 Thanks for submitting this PR so quickly! Would love to get this indicator added. See comments in src/mom.jl. It would also be a good idea to add test cases to ensure the functions work as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants