-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
DOC: fix conflicting ewma documentation #4499
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
Conversation
@@ -75,7 +75,7 @@ | |||
|
|||
EWMA is sometimes specified using a "span" parameter s, we have have that the | |||
decay parameter \alpha is related to the span as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should these stray \alpha
s really be :math:
\alpha``?
Good point! I've updated it so they're all mathified :-) |
excellent! |
Just noticed #4321 which should be covered by these fixes too (it only updates the docs). |
are the docs existing docs just inconsistent with the code (before this change / #4321?) |
Yep the docs are inconsistent right now but how they are fixed depends on whether you define I've made these changes with neww = 1. / (1. + com)
oldw = 1. - neww
...
output[i] = oldw * prev + neww * cur Given the definition for
as they do with this fix, and the fix in #4321. As I said you can do it all the other way around by defining |
@Komnomnomnom ok...but your changes make this consistent (with the def of alpha that you are showing)...so that is fine (you might want to mention that it can be done the other way too...., but not go into detail) |
Ok I added a very short note to the docs. |
@Komnomnomnom merge? |
Yep, go for it. If someone wants to change the wording or anything feel free. |
DOC: fix conflicting ewma documentation
The information in the docs, in the parameter description and in the body of the docstring is conflicting. This should hopefully make them consistent amongst themselves (and with the code).