-
-
Notifications
You must be signed in to change notification settings - Fork 46.2k
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
Adds exponential moving average algorithm #10273
Conversation
|
||
|
||
def exponential_moving_average(series: list[float], window_size: int) -> list[float]: | ||
""" | ||
Returns the exponential moving average of the given array list | ||
>>> exponential_moving_average([2, 5, 3, 8.2, 6, 9, 10], 3) |
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.
The financial calculations seem artificial and trivial when they are supplied with all the data upfront.
Could we convert series: list[float]
--> series: Iterable[float]
so that we could continuously create/add to an EMA as new data came in from some generator expression? We will no longer be able to do len(series)
but will other issues arise?
def exponential_moving_average(series: list[float], window_size: int) -> list[float]: | |
""" | |
Returns the exponential moving average of the given array list | |
>>> exponential_moving_average([2, 5, 3, 8.2, 6, 9, 10], 3) | |
from collections.abc import Iterable | |
def exponential_moving_average(series: Iteratable[float], window_size: int) -> list[float]: | |
""" | |
Returns the exponential moving average of the given array list | |
>>> exponential_moving_average(iter([2, 5, 3, 8.2, 6, 9, 10]), 3) |
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.
Instead of return list[float]
could we consider yielding floats as we finish windows?
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.
Thanks for the suggestion!
I have added generator as parameter instead of an iterator which acts as mock for file or data stream.
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.
Click here to look at the relevant links ⬇️
🔗 Relevant Links
Repository:
Python:
Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper
commands and options
algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper review
to trigger the checks for only added pull request files@algorithms-keeper review-all
to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
|
||
doctest.testmod() | ||
|
||
def test_gen_func(arr: list[float]): |
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.
As there is no test file in this pull request nor any test function or class in the file financial/exponential_moving_average.py
, please provide doctest for the function test_gen_func
Please provide return type hint for the function: test_gen_func
. If the function does not return a value, please provide the type hint as: def function() -> None:
t = 0 | ||
|
||
# Exponential average at timestamp t | ||
st = None |
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.
st = None | |
st = 0 |
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.
Thanks!
@@ -0,0 +1,72 @@ | |||
""" | |||
Calculates exponential moving average (EMA) on the series of numbers |
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.
Active tense
Calculates exponential moving average (EMA) on the series of numbers | |
Calculate the exponential moving average (EMA) on the series of numbers. |
""" | ||
Calculates exponential moving average (EMA) on the series of numbers | ||
Wikipedia Reference: https://en.wikipedia.org/wiki/Exponential_smoothing | ||
Reference: https://www.investopedia.com/terms/e/ema.asp#toc-what-is-an-exponential-moving-average-ema |
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.
Lines of Python code should be max 88 characters so let's not overdo it.
Reference: https://www.investopedia.com/terms/e/ema.asp#toc-what-is-an-exponential-moving-average-ema | |
https://www.investopedia.com/terms/e/ema.asp#toc-what-is-an-exponential-moving-average-ema |
""" | ||
Returns the generator which generates exponential moving average of the given | ||
series generator | ||
>>> list(exponential_moving_average((ele for ele in [2, 5, 3, 8.2, 6, 9, 10]), 3)) |
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.
Just easier to read...
>>> list(exponential_moving_average((ele for ele in [2, 5, 3, 8.2, 6, 9, 10]), 3)) | |
>>> list(exponential_moving_average(iter([2, 5, 3, 8.2, 6, 9, 10]), 3)) |
alpha = 2 / (1 + window_size) | ||
|
||
# Defining timestamp t | ||
t = 0 |
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.
t = 0 | |
timestamp = 0 |
Or maybe even better, ticks
.
t = 0 | ||
|
||
# Exponential average at timestamp t | ||
st = 0.0 |
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.
st = 0.0 | |
exponential_average = 0.0 |
|
||
|
||
def exponential_moving_average( | ||
series_generator: Iterator[float], window_size: int |
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.
Just plain series
is fine.
series_generator: Iterator[float], window_size: int | |
series: Iterator[float], window_size: int |
test_generator = (ele for ele in test_series) | ||
test_window_size = 3 | ||
result = exponential_moving_average(test_generator, test_window_size) |
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.
test_generator = (ele for ele in test_series) | |
test_window_size = 3 | |
result = exponential_moving_average(test_generator, test_window_size) | |
test_window_size = 3 | |
result = exponential_moving_average(series=iter(test_series), window_size=test_window_size) |
Final edits: ce8ecce |
* Adds exponential moving average algorithm * code clean up * spell correction * Modifies I/O types of function * Replaces generator function * Resolved mypy type error * readibility of code and documentation * Update exponential_moving_average.py --------- Co-authored-by: Christian Clauss <cclauss@me.com>
Describe your change:
Checklist: