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

Separate Development Dependencies from Production Dependencies #3877

Closed
anilbeycorintis opened this issue Nov 21, 2024 · 3 comments · Fixed by #3817
Closed

Separate Development Dependencies from Production Dependencies #3877

anilbeycorintis opened this issue Nov 21, 2024 · 3 comments · Fixed by #3817

Comments

@anilbeycorintis
Copy link

Hi,

Thank you for your work on Firedrake!

I noticed that requirements-ext.txt includes both production dependencies (e.g., scipy, sympy) and development dependencies (e.g., pylint, pytest, nbval). This forces users to install unnecessary dev tools, leading to overhead and conflicts.

https://github.com/firedrakeproject/firedrake/blob/76d8daa0132e5067bae17d05985401b64f443acd/requirements-ext.txt#L1:L15

Could you split these into requirements.txt (for production) and requirements-dev.txt, or use extras_require (e.g., pip install firedrake[dev])? Users of Firedrake shouldn’t have to depend on tools like pytest or pylint or nbval.

Thanks

Example: how pytest solves this problem

It is a common practice to separate development requirements. For example, pytest handles this by listing production dependencies separately from development dependencies. Users only need the production list to use pytest. Development dependencies are installed separately, typically in a different environment, only if contributing to pytest.

This approach could help make dependency management cleaner for Firedrake users. Let me know if I can assist!

dependencies = [
    "colorama; sys_platform=='win32'",
    "exceptiongroup>=1.0.0rc8; python_version<'3.11'",
    "iniconfig",
    "packaging",
    "pluggy>=1.5,<2",
    "tomli>=1; python_version<'3.11'",
]
optional-dependencies.dev = [
    "argcomplete",
    "attrs>=19.2",
    "hypothesis>=3.56",
    "mock",
    "pygments>=2.7.2",
    "requests",
    "setuptools",
    "xmlschema",
]

https://github.com/pytest-dev/pytest/blob/72f17d10d76e5260906c17f279c5fa6bd29b5a8d/pyproject.toml#L49:L66

@connorjward
Copy link
Contributor

We are currently in the process of undoing Firedrake's bespoke installation with a much more typical pip-based approach. This includes adding optional dependencies in the way that you suggest.

Note that this new install method will not be the default for a little while (~1 month) as there are more things we need to do before removing firedrake-install.

@dham
Copy link
Member

dham commented Nov 22, 2024

Note that the development tools that are there are pretty lightweight compared to the operational requirements of Firedrake (which, don't forget, also include a full compiler stack to support runtime code generation). pytest, for example is required to check that firedrake built correctly so basically all users need that. We're really talking very minor cases of purely development tools that get installed.

@anilbeycorintis
Copy link
Author

Brilliant! I’m looking forward to adopting the pip-based approach. Thanks so much for your effort!

Just to highlight the importance of the issue: installing nbval alone brings in 55 python packages that are not necessary to run firedrake. They are not purely development tools either. Each unnecessary package is a potential source of conflict (and a security risk). A portion of issues reported in our system are caused by dependency conflicts.

❯ uv add nbval                                                                                                                                                                                          ─╯
Resolved 55 packages in 2.74s

Here is a subset of dependencies that, as a user, I would have preferred to avoid installing. Having the option to choose dependencies selectively in Firedrake (and similarly in pyadjoint, pyop2) would be highly appreciated.

├── cached-property v2.0.1
├── nbval v0.11.0
│   ├── coverage v7.6.8
│   ├── ipykernel v6.29.5
│   │   ├── comm v0.2.2
│   │   │   └── traitlets v5.14.3
│   │   ├── debugpy v1.8.9
│   │   ├── ipython v8.29.0
│   │   │   ├── decorator v5.1.1
│   │   │   ├── jedi v0.19.2
│   │   │   │   └── parso v0.8.4
│   │   │   ├── matplotlib-inline v0.1.7
│   │   │   │   └── traitlets v5.14.3
│   │   │   ├── pexpect v4.9.0
│   │   │   │   └── ptyprocess v0.7.0
│   │   │   ├── prompt-toolkit v3.0.48
│   │   │   │   └── wcwidth v0.2.13
│   │   │   ├── pygments v2.18.0
│   │   │   ├── stack-data v0.6.3
│   │   │   │   ├── asttokens v2.4.1
│   │   │   │   │   └── six v1.16.0
│   │   │   │   ├── executing v2.1.0
│   │   │   │   └── pure-eval v0.2.3
│   │   │   ├── traitlets v5.14.3
│   │   │   └── typing-extensions v4.12.2
│   │   ├── jupyter-client v8.6.3
│   │   │   ├── jupyter-core v5.7.2
│   │   │   │   ├── platformdirs v4.3.6
│   │   │   │   └── traitlets v5.14.3
│   │   │   ├── python-dateutil v2.9.0.post0
│   │   │   │   └── six v1.16.0
│   │   │   ├── pyzmq v26.2.0
│   │   │   ├── tornado v6.4.2
│   │   │   └── traitlets v5.14.3
│   │   ├── jupyter-core v5.7.2 (*)
│   │   ├── matplotlib-inline v0.1.7 (*)
│   │   ├── nest-asyncio v1.6.0
│   │   ├── packaging v24.2
│   │   ├── psutil v6.1.0
│   │   ├── pyzmq v26.2.0
│   │   ├── tornado v6.4.2
│   │   └── traitlets v5.14.3
│   ├── jupyter-client v8.6.3 (*)
│   ├── nbformat v5.10.4
│   │   ├── fastjsonschema v2.20.0
│   │   ├── jsonschema v4.23.0
│   │   │   ├── attrs v24.2.0
│   │   │   ├── jsonschema-specifications v2024.10.1
│   │   │   │   └── referencing v0.35.1
│   │   │   │       ├── attrs v24.2.0
│   │   │   │       └── rpds-py v0.21.0
│   │   │   ├── referencing v0.35.1 (*)
│   │   │   └── rpds-py v0.21.0
│   │   ├── jupyter-core v5.7.2 (*)
│   │   └── traitlets v5.14.3
│   └── pytest v8.3.3
│       ├── iniconfig v2.0.0
│       ├── packaging v24.2
│       └── pluggy v1.5.0
├── pylint v3.3.1
│   ├── astroid v3.3.5
│   ├── dill v0.3.9
│   ├── isort v5.13.2
│   ├── mccabe v0.7.0
│   ├── platformdirs v4.3.6
│   └── tomlkit v0.13.2
├── pytest v8.3.3 (*)
├── pytest-xdist v3.6.1
│   ├── execnet v2.1.1
│   └── pytest v8.3.3 (*)

Thank you again for considering this.

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

Successfully merging a pull request may close this issue.

3 participants