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

Make jupyter a dev-only dependency #2457

Closed
valentinsulzer opened this issue Nov 15, 2022 · 13 comments · Fixed by #2846
Closed

Make jupyter a dev-only dependency #2457

valentinsulzer opened this issue Nov 15, 2022 · 13 comments · Fixed by #2846
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours

Comments

@valentinsulzer
Copy link
Member

There are some issues with dependencies on arm64 (qtconsole)

@valentinsulzer
Copy link
Member Author

Further, with the exception of numpy, all of our dependencies could be "optional" (i.e. some features don't work if you don't have them installed). For most dependencies this is easy (e.g. anytree, imageio), for others this will require some refactoring (casadi).

The goal eventually is to have a pybamm-lite package that only installs numpy and pybamm, and a pybamm that installs all the optional dependencies

@valentinsulzer valentinsulzer added the difficulty: easy A good issue for someone new. Can be done in a few hours label Nov 20, 2022
@ayeankit
Copy link
Contributor

hey @tinosulzer, I want to work in this issue. Can you please assign me?

@deepeshaburse
Copy link
Contributor

Hi @tinosulzer, can I work on this issue?

@deepeshaburse
Copy link
Contributor

deepeshaburse commented Mar 5, 2023

@tinosulzer I was looking through some solutions. I was thinking we could make a separate dev-requirements.txt and add jupyter to it (and remove it from requirements.txt). I could also make the required changes in the README. Would that be the right approach to solve this issue? There are other solutions like Poetry and pyenv, but it will only add more dependencies to the project.

@deepeshaburse
Copy link
Contributor

@Saransh-cpp should I go ahead with this?

@Saransh-cpp
Copy link
Member

Saransh-cpp commented Mar 7, 2023

This would require removing jupyter completely from requirements.txt and moving it from a required dependency to a dev dependency in setup.py.

From here -

PyBaMM/setup.py

Lines 191 to 209 in f45f4a5

install_requires=[
"numpy>=1.16",
"scipy>=1.3",
"pandas>=0.24",
"anytree>=2.4.3",
"autograd>=1.2",
"scikit-fem>=0.2.0",
"casadi>=3.5.0",
"imageio>=2.9.0",
"jupyter", # For example notebooks
"pybtex>=0.24.0",
"sympy>=1.8",
"bpx",
# Note: Matplotlib is loaded for debug plots, but to ensure pybamm runs
# on systems without an attached display, it should never be imported
# outside of plot() methods.
# Should not be imported
"matplotlib>=2.0",
],

to here -

PyBaMM/setup.py

Lines 218 to 221 in f45f4a5

"dev": [
"pre-commit", # For code style checking
"black", # For code style auto-formatting
],

I remember there was some discussion about creating more optional dependency groups (for instance notebooks and plot, just like dev), but I think the message vanished in slack. You might have to wait till there is a consensus on this 🙂

@deepeshaburse
Copy link
Contributor

This would require removing jupyter completely from requirements.txt and moving it from a required dependency to a dev dependency in setup.py.

From here -

PyBaMM/setup.py

Lines 191 to 209 in f45f4a5

install_requires=[
"numpy>=1.16",
"scipy>=1.3",
"pandas>=0.24",
"anytree>=2.4.3",
"autograd>=1.2",
"scikit-fem>=0.2.0",
"casadi>=3.5.0",
"imageio>=2.9.0",
"jupyter", # For example notebooks
"pybtex>=0.24.0",
"sympy>=1.8",
"bpx",
# Note: Matplotlib is loaded for debug plots, but to ensure pybamm runs
# on systems without an attached display, it should never be imported
# outside of plot() methods.
# Should not be imported
"matplotlib>=2.0",
],

to here -

PyBaMM/setup.py

Lines 218 to 221 in f45f4a5

"dev": [
"pre-commit", # For code style checking
"black", # For code style auto-formatting
],

I remember there was some discussion about creating more optional dependency groups (for instance notebooks and plot, just like dev), but I think the message vanished in slack. You might have to wait till there is a consensus on this 🙂

Sure, thanks for letting me know ☺️

@valentinsulzer
Copy link
Member Author

Happy to proceed with just moving jupyter to the dev for now. This the largest and most optional dependency so it makes sense to just do that one first

@valentinsulzer
Copy link
Member Author

@arjxn-py
Copy link
Member

If no one is working on this issue, may I look into this and try to do it? @tinosulzer @Saransh-cpp

@Saransh-cpp
Copy link
Member

Let's wait for @deepeshaburse's response for a couple of days. If there is no activity then you can take this up.

@arjxn-py
Copy link
Member

@Saransh-cpp I believe she's not responding

arjxn-py added a commit to arjxn-py/PyBaMM that referenced this issue Apr 1, 2023
@arjxn-py
Copy link
Member

arjxn-py commented Apr 1, 2023

@Saransh-cpp I have opened this PR trying to solve this issue but I am getting some failing tests, I would be glad if any lead could be provided.

arjxn-py added a commit to arjxn-py/PyBaMM that referenced this issue Apr 1, 2023
valentinsulzer added a commit that referenced this issue Apr 3, 2023
…-only-dependency

Issue #2457 make jupyter a dev only dependency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours
Projects
None yet
5 participants