Skip to content

Conversation

@moagstar
Copy link

@moagstar moagstar commented Feb 4, 2025

Fix for #976

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

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@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
Contributor

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
@codspeed-hq
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
Contributor

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.

Comment on lines +1 to +2
"""Methods for Fit, Predict, Forecast (fast), Cross Validation and plotting"""

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""Methods for Fit, Predict, Forecast (fast), Cross Validation and plotting"""

Copy link
Contributor

Choose a reason for hiding this comment

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

This should fix the linter error, I don't have permission to push to your branch.

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