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

breaking: remove deprecated behavior #1220

Merged
merged 9 commits into from
Dec 19, 2024
Merged

breaking: remove deprecated behavior #1220

merged 9 commits into from
Dec 19, 2024

Conversation

jmoralez
Copy link
Member

@jmoralez jmoralez commented Nov 27, 2024

Removes the following behavior, which has been deprecated for about a year:

  • Providing the id as the dataframe index, it has to be a column. Also the outputs from the methods will now always have the id as a column.
  • Providing sort_df to fit and cross_validation, we always check if it's sorted.
  • Providing num_workers_loader to the models constructor, those should now be provided through dataloader_kwargs.

Also updates the documentation to remove usages of this behavior, docs that use datasetsforecast.losses and uses utilsforecast.plotting.plot_series instead of StatsForecast.plot

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@jmoralez jmoralez marked this pull request as ready for review November 27, 2024 19:22
Copy link
Contributor

@elephaint elephaint left a comment

Choose a reason for hiding this comment

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

Mainly non-blocking comments, but the following notebooks in tutorials still have os.environ['NIXTLA_ID_AS_COL'] in it:

  • 02_cross_validation
  • 10_distributed
  • 12_using_mlflow
  • 19_large_datasets
  • 20_conformal_prediction

nbs/core.ipynb Show resolved Hide resolved
nbs/docs/capabilities/04_hyperparameter_tuning.ipynb Outdated Show resolved Hide resolved
nbs/docs/capabilities/05_predictInsample.ipynb Outdated Show resolved Hide resolved
nbs/docs/capabilities/06_save_load_models.ipynb Outdated Show resolved Hide resolved
nbs/docs/getting-started/01_introduction.ipynb Outdated Show resolved Hide resolved
nbs/docs/getting-started/05_datarequirements.ipynb Outdated Show resolved Hide resolved
@jmoralez
Copy link
Member Author

jmoralez commented Dec 6, 2024

I removed the uses of datasetsforecast.losses, the ones that set the env variable wouldn't be broken but it can be confusing, so I removed them in 051ec9b. We still have to update all notebooks which have a reset_index in them but I wanted to finish this within my lifetime so I didn't do it here but I'll update this PR when I get time to run those as well.

Copy link
Contributor

@elephaint elephaint left a comment

Choose a reason for hiding this comment

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

Thanks!

@jmoralez jmoralez merged commit 33502b0 into main Dec 19, 2024
17 checks passed
@jmoralez jmoralez deleted the remove-deprecations branch December 19, 2024 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chore: deprecate NIXTLA_ID_AS_COL or issue warning
2 participants