Skip to content
New issue

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

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

Already on GitHub? # to your account

Feat: Add weighted moving average (WMA) #47

Merged
merged 1 commit into from
Oct 21, 2022

Conversation

menkaru
Copy link
Contributor

@menkaru menkaru commented Aug 7, 2021

Hey there, this PR implements the weighted moving average indicator, which assigns weights that decrease linearly (n, n-1, etc) as opposed to exponentially as in the EMA.

From what I can see the WMA seems to be a requirement to implement the hull moving average (Issue #36), which I'll likely be tackling next.

@menkaru menkaru changed the title Feat: add weighted moving average (WMA) Feat: Add weighted moving average (WMA) Aug 7, 2021
@menkaru
Copy link
Contributor Author

menkaru commented Aug 7, 2021

Update: I finished implementing the hull moving average on this branch. If you'd like I can merge that with this pull request, or just submit a new one later.

Edit: Solves #36

@virtualritz
Copy link

My PR now also includes all three new indicators merged from @menkaru's resp. branches.

@virtualritz
Copy link

@menkaru, is your branch for QuantitativeQualitativeEstimation WIP/unfinished? The indicators::quantitative_qualitative_estimation::tests::test_next for this feature fails for me:

thread 'indicators::quantitative_qualitative_estimation::tests::test_next' panicked at 'assertion failed: `(left == right)`
  left: `(44.52, 24.53)`,
 right: `(46.11, 23.18)`', src/indicators/quantitative_qualitative_estimation.rs:241:9

@menkaru
Copy link
Contributor Author

menkaru commented Nov 23, 2021

@virtualritz, it's been a while but iirc I ran into issue #46 due to all the EMAs required. It's pretty much done, but it just gives slightly different results to the reference I used (pandas_ta) to generate the test cases.

@virtualritz
Copy link

What does "pretty much done" mean? You consider it close enough and won't touch the code or minor tweaks are needed to fix? In the former case we could just update the test case values, I guess?

@menkaru
Copy link
Contributor Author

menkaru commented Nov 23, 2021

Sorry, I should have been more specific. The implementation is correct and complete. From what I can tell the problem is the same as issue #46, which seems to require structural changes to the library. I considered changing the test cases, but the results didn't seem close enough to warrant it.

For what it's worth, I wouldn't merge it until a fix for #46 is decided on.

@greyblake greyblake merged commit 93dcca4 into greyblake:master Oct 21, 2022
@greyblake
Copy link
Owner

@menkaru Sorry, it took way too long.. Thanks for the PR!

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

Successfully merging this pull request may close these issues.

3 participants