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

Splitting the required dependencies into optional dependencies #3035

Closed
arjxn-py opened this issue Jun 14, 2023 · 12 comments · Fixed by #3044
Closed

Splitting the required dependencies into optional dependencies #3035

arjxn-py opened this issue Jun 14, 2023 · 12 comments · Fixed by #3044
Assignees
Labels
difficulty: medium Will take a few days feature

Comments

@arjxn-py
Copy link
Member

arjxn-py commented Jun 14, 2023

Description

Currently pybamm is installing many dependencies we want to have a lite version of pybamm by splitting the required deps into optional deps

@Saransh-cpp
Copy link
Member

Related to #2457

@Saransh-cpp
Copy link
Member

Discussion from today's meeting -

  • Making dependencies like imageio optional like jupyter
    • Adding multiple small optional tags like [gif] for imageio
    • Adding a single big tag to cover all the additional dependencies, like [all_optional]
    • Visualise dependencies using https://pypi.org/project/pipdeptree/

Maintainers can iteratively add here the dependencies which they think can be made optional.

@Saransh-cpp Saransh-cpp added the difficulty: medium Will take a few days label Jun 14, 2023
@valentinsulzer
Copy link
Member

The following dependencies can be made optional without affecting 95% of functionality:

  • pandas
  • anytree
  • autograd
  • scikit-fem
  • pybtex
  • sympy

@agriyakhetarpal
Copy link
Member

The dependency graph with pipdeptree is also related to the documentation project, I may pick that up later if that's okay?

@arjxn-py
Copy link
Member Author

The following dependencies can be made optional without affecting 95% of functionality

How should I go with grouping them further or should we have a single tag say dev?

@brosaplanella
Copy link
Member

The following dependencies can be made optional without affecting 95% of functionality:

  • pandas

About pandas, I think the trickiest point will be that some parameter sets won't work without pandas (they use read_csv). One way around would be to hardcode the csv files in the Python script for the given parameter.

Just to clarify, I am not against pandas being optional, but I think it would be good to keep all parameters working with the basic PyBaMM.

@Saransh-cpp
Copy link
Member

Saransh-cpp commented Jun 14, 2023

Please feel free to update this. We'll also need documentation on this so this can be worked upon in multiple PRs.

Easy

Should be straight forward and we do want them to be optional.

  • imageio - [gif]
  • anytree - ?
  • autograd - ?
  • scikit-fem - ?
  • pybtex - [cite]
  • sympy - [latexify]
  • We should also add jax and jaxlib here - [jax]
  • We might as well as add scikits.odes - [odes] (just assume that the user has SUNDIALS set up, otherwise let the pip install pybamm[odes] fail)
  • tqdm - [tqdm]
  • bpx - [bpx]

Moderate

Might require some work but we still might want them to be optional.

  • pandas
  • matplotlib - [plot]

Hard

We might not even want to change their status and keep them as the required ones.

  • scipy
  • casadi
  • xarray

Keep as required

  • numpy
  • scipy
  • casadi
  • xarray

@valentinsulzer
Copy link
Member

For pandas we could set the default to use the equivalent numpy function and then we would not need pandas

I would actually put tqdm and bpx in the easy bucket too

For the hard ones:

  • scipy: impossible to get rid of if solving PDEs (it is used in the discretization)
  • casadi: very hard to get rid of and would lead to performance regressions
  • xarray: impossible to get rid of without having annoying-to-maintain try-excepts in the ProcessedVariable class

@Saransh-cpp
Copy link
Member

How should I go with grouping them further or should we have a single tag say dev?

There should be smaller groups, like [plot] for matplotlib and imageio and [jax] for jax and jaxlib, and a larger group, like [optionals] with every optional library included (maybe not the ones that are OS specific).

@valentinsulzer
Copy link
Member

I really like the way pandas handles and documents optional dependencies: https://pandas.pydata.org/docs/getting_started/install.html#optional-dependencies

So pybamm[all] would install everything.

I'm not sure whether zsh handles square brackets so we might need to encourage using quotes e.g. pip install "pybamm[all]" (as pandas does)

@arjxn-py
Copy link
Member Author

Pandas handles this type of installation using pyproject.toml introduced in PEP 518 but our build-system is based on setup.py which I'd also need to figure out to migrate to pyproject.toml

@Saransh-cpp
Copy link
Member

You don't necessarily need pyproject.toml to create optional dependencies. Migrating to pyproject.toml would be a bit tricky (we've tried once 🙂), but yes, very much appreciated. I'm not sure which build backend is used by pandas but you can look into setuptools, scikit-build, and meson-py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium Will take a few days feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants