Skip to content

Add Expanding and EWM.mean #412

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

Merged
merged 5 commits into from
Apr 5, 2021

Conversation

mroeschke
Copy link
Contributor

@mroeschke mroeschke commented Mar 22, 2021

xref #220

Added expanding and ewm ops. For ewm, I used pandas' ewm implementation.

Copy link

@jreback jreback left a comment

Choose a reason for hiding this comment

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

@mroeschke if u can add some tests esp to assert the NotImplemented

@mroeschke mroeschke changed the title WIP: Add Expanding and EWM.mean Add Expanding and EWM.mean Mar 29, 2021
raise ValueError("alpha must satisfy: 0 < alpha <= 1")
com = (1 - alpha) / alpha
else:
raise ValueError("Must pass one of com, span, halflife, or alpha")
Copy link

Choose a reason for hiding this comment

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

are these all hit in tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added tests for these.

@ramicaza
Copy link

ramicaza commented Mar 31, 2021

I started working on implementing EWMA today then halfway through I realized "hey I should probably check if there were any PRs in the last month since I last checked". Then I found this and threw away what I had haha

Nice work with this :) I think useful next steps would be adding nan handling, min_periods, and std. Do you have any plans to implement these? If not I can give it a stab.

@mroeschke
Copy link
Contributor Author

Agreed, and it would be great if you could follow up with those features @ramicaza! I don't think I'll be able to tackle that if/when this PR is merged in.

@mroeschke
Copy link
Contributor Author

@martindurant ready for a look when you get the chance (the failing test seems unrelated test_kafka_batch_checkpointing_async_nodes_1)

@martindurant
Copy link
Member

Agreed, same as in #411

@martindurant martindurant merged commit ae84982 into python-streamz:master Apr 5, 2021
@mroeschke mroeschke deleted the feature/ewm branch April 5, 2021 17:24
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.

4 participants