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.

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

feat(fractional_differencing): create frac diff plugin #151

Merged
merged 2 commits into from
Dec 20, 2023

Conversation

ngriffiths13
Copy link
Collaborator

@ngriffiths13 ngriffiths13 commented Dec 19, 2023

The transformer implementation of frac diff was also replaced with the plugin implementation. The api did not change however.

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Fractional differencing plugin

Any other comments?

The transformer implementation of frac diff was also replaced with the plugin implementation. The api did not change however.
@topher-lo topher-lo added the enhancement New feature or request label Dec 20, 2023
.gitignore Outdated Show resolved Hide resolved
Copy link
Collaborator

@baggiponte baggiponte left a comment

Choose a reason for hiding this comment

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

Nothing to add about the implementation. Kudos for writing the docstring as well. Could it be possible to add some tests too?

@abstractqqq
Copy link
Collaborator

abstractqqq commented Dec 20, 2023

I agree. Let's add one test and have this approved.

The error in test code is interesting though:
ModuleNotFoundError: No module named 'aeon.transformations.series'

I don't think this is related to this PR. But does anybody know what is aeon?

@ngriffiths13
Copy link
Collaborator Author

Nothing to add about the implementation. Kudos for writing the docstring as well. Could it be possible to add some tests too?

So there were already tests here from the previous implementation. They still pass. Do you want me to write an explicit test for the plugin? The transformer that uses the plugin is tested

@baggiponte
Copy link
Collaborator

I don't think this is related to this PR. But does anybody know what is aeon?

It's a fork of functime.

@baggiponte
Copy link
Collaborator

LGTM :)

@ngriffiths13 ngriffiths13 merged commit 6324d19 into main Dec 20, 2023
4 checks passed
@ngriffiths13 ngriffiths13 deleted the feat/frac-diff-plugin branch December 20, 2023 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants