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

Why pin runtime dependencies so tightly? #11

Closed
MarcoGorelli opened this issue May 24, 2021 · 15 comments
Closed

Why pin runtime dependencies so tightly? #11

MarcoGorelli opened this issue May 24, 2021 · 15 comments

Comments

@MarcoGorelli
Copy link

Hi,

Looking at the setup.py file, it looks like the following are all required runtime dependencies, all of which need to be pinned very precisely:

requirements = [    "Cython==0.29.23",    "cvxpy==1.1.12",    "fbprophet==0.5",    "holidays==0.9.10",  # 0.10.2,    "ipykernel==4.8.2",    "ipython==7.1.1",    "ipywidgets==7.2.1",    "jupyter==1.0.0",    "jupyter-client==6.1.5",    "jupyter-console==6.",  # used version 6 to avoid conflict with ipython version    "jupyter-core==4.7.1",    "matplotlib==3.4.1",    "nbformat==5.1.3",    "notebook==5.4.1",    "numpy==1.20.2",    "osqp==0.6.1",    "overrides==2.8.0",    "pandas==1.1.3",    "patsy==0.5.1",    "Pillow==8.0.1",    "plotly==3.10.0",    "pystan==2.18.0.0",    "pyzmq==22.0.3",    "scipy==1.5.4",    "seaborn==0.9.0",    "six==1.15.0",    "scikit-learn==0.24.1",    "Sphinx==3.2.1",    "sphinx-gallery==0.6.1",    "sphinx-rtd-theme==0.4.2",    "statsmodels==0.12.2",    "testfixtures==6.14.2",    "tornado==5.1.1",    "tqdm==4.52.0"]

My question is - why pin them so tightly, and are all of them really necessary? E.g. do I really need sphinx-gallery? Such tight pins make it very difficult to integrate into any existing project. Why not just require a lower bound for many/most of these?

@Reza1317
Copy link
Contributor

Hi Marco,

That is a reasonable comment. We wanted to avoid installation issues (when library is installed on its own). Some of which were coming from the requirements for Prophet which we might make optional or remove in future releases. We will also see which requirements are possible to be made less strict. Thanks for the input.

Reza

@MichalChromcak
Copy link

MichalChromcak commented May 31, 2021

Hi Reza,

We recently received a request to add a wrapper for GreyKite as a part of HCrystalBall. I plan to update the new release to prophet>=1.0 and also searching through your codebase, it seems, the migration might be not such a difficult move?

Could you let me know if both - the addition to HCrystalBall is welcomed and whether you'd directly consider prophet 1 over e.g. fbprophet 0.6 in case you would consider updating the dependencies? Would make my life much easier to know.

Or maybe leave the prophet as an optional dependency?

Michal

@husafan
Copy link

husafan commented Jun 1, 2021

This is causing problems for me as well.

@wilfreddesert
Copy link

I am not sure whether it's related, but trying to do pip install greykite broke my Jupyter installation for me

@MarcoGorelli
Copy link
Author

MarcoGorelli commented Jun 2, 2021

yeah I think as it currently stands the only way to use greykite is to make a new conda environment / virtualenv specially for it :)

@CatChenal
Copy link

Why is pystan in the requirements?

@husafan
Copy link

husafan commented Jun 3, 2021

@CatChenal - I know PyStan is a prerequisite for Prophet: https://facebook.github.io/prophet/docs/installation.html

@Reza1317
Copy link
Contributor

Reza1317 commented Jun 4, 2021

Yes, I think most of these issues are related to prophet installation. We are working on a solution and it should be resolved in the next release which we hope to publish in a timely fashion. Please stay tuned for an update.

@aks43725
Copy link

As an extended question, is there extended usage of FBProphet included? I only see a wrapper class to run Prophet models (yet not the most recent version), which can totally be done by directly calling the Prophet package independently, but didn't find any extended usage. If that's the case, including (FB)Prophet as part of the requirements seems redundant, as it causes major installation issues for this awesome package.

@ahgraber
Copy link

Will the next version move from fbprophet to prophet, following the prophet library's name migration?

@KaixuYang
Copy link
Contributor

Hi everyone,

Thanks for the comments in this thread. We have just released greykite==0.2.0 on Pypi which

  • removes the dependency on fbprophet (it is optional now, but you need to install it manually if you want to use the Greykite wrapper on fbprophet)
  • removes dependencies that are not useful during runtime.
  • changes pinned dependencies to ranged dependencies.

Hopefully this resolves the installation issues.

@KaixuYang
Copy link
Contributor

@aks43725 The prophet wrapper is an estimator wrapper, allowing it to be used in the Greykite pipeline (including cross-validation, trai/test split, etc.) We changed the fbprophet to optional so you don't need it to install Greykite anymore. And if you have fbprophet==0.5, 0.6, 0.7 it will be compatible, but without some later added parameters in 0.6 or 0.7.

@KaixuYang
Copy link
Contributor

@ahgraber Currently we unpinned fbprophet and the library works with fbprophet==0.5 ,0.6, 0.7 except for a small fraction of later added parameters in fbprophet==0.6, 0.7. We currently don't support prophet==1.0 because it has a different library name. In the long term we might want to support prophet==1.0, but this may not happen in the near future.

@KaixuYang
Copy link
Contributor

Please see greykite==0.2.0. This issue is resolved. Thanks everyone!

@ahgraber
Copy link

ahgraber commented Jul 1, 2021

@ahgraber Currently we unpinned fbprophet and the library works with fbprophet==0.5 ,0.6, 0.7 except for a small fraction of later added parameters in fbprophet==0.6, 0.7. We currently don't support prophet==1.0 because it has a different library name. In the long term we might want to support prophet==1.0, but this may not happen in the near future.

Thanks for the update. I believe the limitation on prophet==1.0 will prevent greykite + prophet compatibility with python 3.9. Regardless, looking forward to this update!

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

No branches or pull requests

9 participants