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

fix: ensure statsforecast can run with PYTHONOPTIMIZE=2 #977

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

moagstar
Copy link

@moagstar moagstar commented Feb 4, 2025

Fix for #976

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 Feb 4, 2025

CLA assistant check
All committers have signed the CLA.

@moagstar moagstar force-pushed the fix/allow-pythonoptimize-2 branch 3 times, most recently from caa2e56 to c6364cd Compare February 4, 2025 09:21
@jmoralez
Copy link
Member

jmoralez commented Feb 5, 2025

Thanks! Can you please export the notebooks (run nbdev_export)?

Alternatively you can just install the pre-commit hooks as described in our contributing guide and that'll export them.

@moagstar moagstar force-pushed the fix/allow-pythonoptimize-2 branch from c6364cd to 862b2ed Compare February 5, 2025 22:06
@moagstar
Copy link
Author

moagstar commented Feb 5, 2025

@jmoralez ah yes sorry I missed that step - I had some trouble with the pre-commit hooks, but I've managed to generate the files manually now.

@jmoralez jmoralez linked an issue Feb 6, 2025 that may be closed by this pull request
@jmoralez jmoralez added the fix label Feb 6, 2025
Copy link

codspeed-hq bot commented Feb 6, 2025

CodSpeed Performance Report

Merging #977 will not alter performance

Comparing crunchr:fix/allow-pythonoptimize-2 (862b2ed) with main (52a5c9a)

Summary

✅ 6 untouched benchmarks

@moagstar
Copy link
Author

moagstar commented Feb 6, 2025

@jmoralez I see that some checks are failing.

nbdev_export.............................................................Failed
- hook id: nbdev_export
- files were modified by this hook
Error: Process completed with exit code 1.

When I ran nbdev locally it did generate some python files for notebooks I hadn't touched - but one of those generated files had some mypy errors:

python/statsforecast/distributed/multiprocess.py:35: error: Unexpected keyword argument "df" for "_StatsForecast"  [call-arg]
python/statsforecast/distributed/multiprocess.py:47: error: Unexpected keyword argument "df" for "_StatsForecast"  [call-arg]

So I just commit the python files for the two notebooks I had touched. I guess the fact that nbdev is generating more files is the reason for the CI failure?

Do you have some idea what might be happening here?

@jmoralez
Copy link
Member

jmoralez commented Feb 6, 2025

We use a specific version of nbdev

dev_requirements = black datasetsforecast fire nbdev==2.3.25 nbformat nbdev_plotly pandas[plot] pmdarima polars[numpy] pre-commit prophet pyarrow pybind11 pytest scikit-learn setuptools<70 supersmoother yfinance

so you have to set up the environment as described in the contributing guidelines, or I can fix the linting errors if you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not possible to run statsforecast with PYTHONOPTIMIZE=2
3 participants