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

migrate core processing to utilsforecast #656

Merged
merged 17 commits into from
Nov 13, 2023
Merged

migrate core processing to utilsforecast #656

merged 17 commits into from
Nov 13, 2023

Conversation

jmoralez
Copy link
Member

@jmoralez jmoralez commented Oct 2, 2023

  • Bumps the min python version to 3.8
  • Moves polars to be an optional dependency
  • Adds deprecation warnings to passing df to the StatsForecast constructor.
  • Makes use of functions from utilsforecast to perform the processing.
  • Uses the base grouped array from utilsforecast to avoid some code duplication.
  • Deprecates the sort_df argument (in utilsforecast we sort if needed).
  • Only saves the original dates and times if we'll compute the fitted values.
  • Removes the warnings about ds being object and tries to parse it as it previously did.
  • Removes the usage of df in the constructor in the fugue methods.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@jmoralez jmoralez requested a review from AzulGarza November 10, 2023 22:53
@jmoralez jmoralez marked this pull request as ready for review November 10, 2023 22:55
Copy link
Member

@AzulGarza AzulGarza left a comment

Choose a reason for hiding this comment

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

This is great, @jmoralez! Abstracting GroupedArray in utilsforecast is a really cool effort. The code now is more navigable. 🎉

I just had a question regarding using unique_id as an index.

statsforecast/core.py Show resolved Hide resolved
statsforecast/core.py Show resolved Hide resolved
statsforecast/core.py Show resolved Hide resolved
statsforecast/core.py Show resolved Hide resolved
@AzulGarza AzulGarza self-requested a review November 11, 2023 03:32
Copy link
Member

@AzulGarza AzulGarza left a comment

Choose a reason for hiding this comment

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

We agreed on maintaining the warnings for more time to allow users to change their code. 🙌

@jmoralez jmoralez merged commit 34b0d79 into main Nov 13, 2023
16 checks passed
@jmoralez jmoralez deleted the utils-proc branch November 13, 2023 17:50
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.

2 participants