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

Augment fourier - dataframes only #182

Merged
merged 5 commits into from
Oct 24, 2023

Conversation

GTimothee
Copy link
Contributor

@GTimothee GTimothee commented Oct 24, 2023

Hi,
Let me know your thoughts about this implementation.
I think it is what was expected. With order 1 (k=1) I got the same results than the current function.
I only implemented the function for the DataFrames, and if it is validated it will be straightforward to do the same for the groups.
I would also like to replace those for loops, but for the moment I was more interested in the correctness of the results :)

fix #22
NB this is a working version. I implemented my fix in a _v2 function so that the other function still works :)

@mdancho84 mdancho84 merged commit 76bd4b6 into business-science:master Oct 24, 2023
mdancho84 added a commit that referenced this pull request Oct 24, 2023
@mdancho84
Copy link
Contributor

OK, I've just made a few updates to help make it more consistent.

max periods was changed to periods.
periods: Union[int, Tuple[int, int], List[int]] = 1 which is consistent with how augment_lags works.

I think it looks pretty good. See if you can do the grouped version.

@mdancho84
Copy link
Contributor

Also, make sure to do a git pull. I've relocated to the feature_engineering submodule.

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.

tk.augment_fourier()
2 participants