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

arima model user guide #570

Merged
merged 66 commits into from
Sep 29, 2023
Merged

arima model user guide #570

merged 66 commits into from
Sep 29, 2023

Conversation

narencastellon
Copy link
Contributor

No description provided.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@CLAassistant
Copy link

CLAassistant commented Jun 25, 2023

CLA assistant check
All committers have signed the CLA.

@kvnkho
Copy link
Collaborator

kvnkho commented Jun 25, 2023

This is amazing @Naren8520! Thanks for the contribution.

I think you need to add your email to your Github account to be able to sign the License agreement: https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-email-preferences/adding-an-email-address-to-your-github-account

Second, I saw a tqdm error in the notebook. If you want to remove it, you can Add a cell at the top like this:

#| hide
import warnings
warnings.simplefilter('ignore')

import logging
logging.getLogger('statsforecast').setLevel(logging.ERROR)

And it will hide the this cell from the HTML file. This will hide the tqdm error.

@kvnkho
Copy link
Collaborator

kvnkho commented Jun 26, 2023

The tutorials have reorganized a bit. Could you put your file under

docs/tutorials/ARIMA.ipynb

?

@mergenthaler
Copy link
Member

@Naren8520: a fantastic job. Thanks!

@FedericoGarza: please share your comments on this sort of text. I'm particularly interested to know if something is missing or if the structure seems right to you. As you know, @Naren8520, has expressed to do the same for other models like ETS, MSTL, and Theta.

@kvnkho
Copy link
Collaborator

kvnkho commented Jul 14, 2023

Hey @Naren8520 , before you add more here. Could we merge this in? It's getting quite big for a review lol.

@kvnkho
Copy link
Collaborator

kvnkho commented Sep 25, 2023

Note to self: check parameters of different models before approving. Some have updated.

@kvnkho
Copy link
Collaborator

kvnkho commented Sep 25, 2023

For future reference, there is something weird with cross-validation plotting. I find this snippet is not printing anything so I will remove it for now and reinsert after investigating.

crossvalidation_df.rename(columns = {'y' : 'actual'}, inplace = True) # rename actual values 

cutoff = crossvalidation_df['cutoff'].unique()

for k in range(len(cutoff)): 
    cv = crossvalidation_df[crossvalidation_df['cutoff'] == cutoff[k]]
    StatsForecast.plot(df, cv.loc[:, cv.columns != 'cutoff'], engine="matplotlib")

@kvnkho
Copy link
Collaborator

kvnkho commented Sep 26, 2023

I finished reviewing this but CI is failing because seaborn is used in a couple of these. plotly is also used in some. Do we need to make a development dependency file? @jmoralez

EDIT: Actually, these files shouldn't be run in CI/CD I think. I will remove them from the CI/CD

@kvnkho
Copy link
Collaborator

kvnkho commented Sep 26, 2023

Ready to merge! @jmoralez @Naren8520

@jmoralez
Copy link
Member

@kvnkho it's good to run these files in the CI to make sure we don't have any broken tutorials. You can add seaborn to the conda section of dev/environment.yml and dev/local_environment.yml (note that these are sorted alphabetically, so it should be added after scipy), plotly is already there so it should be fine.

@jmoralez
Copy link
Member

@kvnkho nevermind. Seems like we don't run any of the documentation. I think you can add a .notest file in docs/models like the one here so they're not run.

@kvnkho
Copy link
Collaborator

kvnkho commented Sep 28, 2023

I think this should be good @FedericoGarza @Naren8520 @jmoralez. I am having problems previewing locally as I get errors in other files.

@kvnkho
Copy link
Collaborator

kvnkho commented Sep 28, 2023

Was able to preview locally. Looks good!

@kvnkho kvnkho merged commit cbcfe25 into Nixtla:main Sep 29, 2023
19 checks passed
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.

5 participants