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.

Sign up for GitHub

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

Jump to bottom

🚀 integrate with tsdownsample #191

Merged
merged 10 commits into from
Apr 14, 2023
Merged

🚀 integrate with tsdownsample #191

merged 10 commits into from
Apr 14, 2023

Conversation

jvdd
Copy link
Member

@jvdd jvdd commented Apr 6, 2023 •
edited

  • avoid creating tsdownsample objects every time we downsample - should be done in the constructor
  • fix serialization -> addressed in 🎉 support serialization tsdownsample#34
  • redefine the size-threshold, given that minmax is now way faster! 🐎
  • verify docs
  • remove MinMaxOverlap?

    I would keep it, but add comments about this aggregator.

  • remove lttbc & the build / setup.py commands
  • update the README.md with the tsdownsample integration! 🔥
  • what about the parallell parameter?

    would add the **downsample_kwargs argument to the datapoint selector impelemntations.

  • Verify that basic-example notebooks are still working.

Copy all Python implementation to tsdownsample - where we can properly test whether these yield the same results

jonasvdd
jonasvdd reviewed Apr 6, 2023
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
@jonasvdd jonasvdd added enhancement New feature or request Optimization Aims to make the toolkit more memory / computationally efficient labels Apr 6, 2023
jvdd
jvdd commented Apr 14, 2023
View reviewed changes
plotly_resampler/aggregation/aggregators.py Outdated
@@ -62,6 +69,7 @@ class LTTB(DataPointSelector):
>>> s = pd.Series(["a", "b", "c", "a"])
>>> cat_type = pd.CategoricalDtype(categories=["b", "c", "a"], ordered=True)
>>> s_cat = s.astype(cat_type)
* `LTTB` has no downsample kwargs, as it cannot be paralellized.
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe plug here MinMaxLTTB?

Copy link
Member

Choose a reason for hiding this comment

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

agree!

jvdd
jvdd commented Apr 14, 2023
View reviewed changes
Copy link
Member Author

@jvdd jvdd left a comment

Choose a reason for hiding this comment

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

Looks good! See comments

@jonasvdd jonasvdd merged commit 061344a into np_array_interface Apr 14, 2023
@jvdd jvdd deleted the tsdownsample branch June 18, 2023 09:23
@jvdd jvdd mentioned this pull request Jul 5, 2023
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonasvdd jonasvdd jonasvdd left review comments

Assignees
No one assigned
Labels
enhancement New feature or request Optimization Aims to make the toolkit more memory / computationally efficient
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jvdd @jonasvdd