Use dependency groups for development and documentation dependencies#2982
Conversation
Remove 'documentation' and 'development' optional dependencies from 'all'. Rename development to dev since uv looks for a 'dev' group to automatically install in development. See: https://docs.astral.sh/uv/concepts/projects/sync/#syncing-development-dependencies
|
I can also update the README with up to date information for how to setup the dev environment with uv |
|
Using dependency groups instead of just declaring the group as optional is a valid change. Updating readme to include instructions to use uv should be done in a seperate pr. |
|
If possible ci changes are preferred in a isolated PR. you can probably make the CI pr first and then update this PR. |
This reverts commit cad4249. Moved this change to a separate PR (pymodbus-dev#2984)
|
Note: the all= seems wrong it no longer installs simulator and pyserial. |
|
@janiversen does it make sense to have Having that as an option would also make it easier if just using pip for development. To install all dependencies in the current state of this PR with pip would be Edit, I just saw your comment:
I can revert the commit removing add as an optional |
|
The current description in readme should still be correct without modification. I am not sure I like the added "--group all", it complicates something that was simple. |
|
Yeah I can simplify it. It would be confusing to have a group and an extra both named diff --git a/README.rst b/README.rst
index 709a7e4c..d20f6cbd 100644
--- a/README.rst
+++ b/README.rst
@@ -216,11 +216,11 @@ or the bleeding edge::
Install required development tools in editable mode::
- pip install -e ".[development]"
+ pip install -e ".[all]" --group dev
Install all (allows creation of documentation etc) in editable mode::
- pip install -e ".[all]"
+ pip install -e ".[all]" --group documentation
.. note::
The use of the ``-e`` (editable) flag is recommended when making changes.I'll also add some more instructions for how to work with uv since it detects the |
|
Using "dev" instead of "development" is quite ok, but when when instructing pip to install "all" it should do so, the "--group" is confusing. it is also confusing that "all" only refers to part of the installation...it is not clear what is part of "all" and what is part of group. I am getting less convinced that using groups actually helps. |
|
I can also organize the dependencies in the [dependency-groups]
dev = [
{include-group = "test"},
{include-group = "lint"},
{include-group = "build"},
{include-group = "type-checking"},
]
test = [
"coverage>=7.14.3",
"pytest>=9.1.1",
"pytest-asyncio>=1.4.0",
"pytest-cov>=7.1.0",
"pytest-profiling>=1.7.0;python_version<'3.13'",
"pytest-timeout>=2.3.1",
"pytest-xdist>=3.6.1",
"pytest-aiohttp>=1.1.1",
]
lint = [
"codespell>=2.4.2",
"pylint>=4.0.6",
"ruff>=0.15.20",
]
build = [
"build>=1.5.0",
"twine>=6.2.0",
]
type-checking = [
"types-Pygments",
"types-pyserial",
"zuban>=0.9.0"
]
documentation = [
"recommonmark>=0.7.1",
"Sphinx>=7.3.7;python_version<'3.12'",
"Sphinx>=9.1.0;python_version>='3.12'",
"sphinx-rtd-theme>=3.1.0"
]Projects like FastAPI do that (https://github.com/fastapi/fastapi/blob/255b912928904e3ba5980425a54d6837c8bd1a1c/pyproject.toml#L122-L187) |
|
Also I see that build is installed. If you have |
|
Let me put it simple...,when instructing pip to install "all" it should do so without extra parameters. also ".[x] --group y" to install x and y are just confusing. no need to split development into smaller groups, either you want to development or not. |
|
I changed To keep it exactly the same, I could also just add The problem with the current docs is that they say |
The main reason to use them is to stop dev only dependencies from being installed from PyPI, like doc dependencies being included in |
|
Again when you write ".[all]" it means ALL so "--group dev" do not make sense. in the current version ".[development]" installs everything needed for development, but if you want to test with pyserial or make documentation that is not enough, which is why you have "all". |
|
But if you specify "all" it is because you want ALL....there are no "all-runtime" it would not make sense. |
|
We currently have: That can be replaced without problems (IF there are a real advantage somewhere) with: But any combination of "pymodbus[x,y,z] --group a,b,c" is just confusing and not something likely to be accepted. |
|
What exactly should |
|
|
Pymodbus[all] means just as it works now, ALL ! It is just a convenience for developers, who typically install everything. For runtime pymodbus only depend on pyserial, so "all" for runtime is just the same as "pyserial". Please do not forget, this is a library not an app !! The comment about package managers worrying about "user-facing" vs "development" think in apps, not in a library. Again I am not against "--group", but I am against the confusion that comes from mixing "pymodbus[x,y,z] with "--group". |
|
It's possible to not have extras and group mixed when using pip. I could make a [dependency-groups]
# ...
dev-all = [
"pymodbus[all]", # serial and simulator
{include-group = "dev"},
{include-group = "documentation"},
]It could also be called
|
|
README would have this to say (no extras and group mixing): diff --git a/README.rst b/README.rst
index 709a7e4c..b9b75f9b 100644
--- a/README.rst
+++ b/README.rst
@@ -174,8 +174,6 @@ Pymodbus offers a number of extra options:
- **serial**, needed for serial communication
- **simulator**, needed by pymodbus.simulator
-- **documentation**, needed to generate documentation
-- **development**, needed for development
- **all**, installs all of the above
which can be installed as::
@@ -216,11 +214,11 @@ or the bleeding edge::
Install required development tools in editable mode::
- pip install -e ".[development]"
+ pip install -e . --group dev
Install all (allows creation of documentation etc) in editable mode::
- pip install -e ".[all]"
+ pip install -e . --group dev-all
.. note::
The use of the ``-e`` (editable) flag is recommended when making changes. |
|
This do not solve the base problem! you forget that pymodbus[pyserial,simulator] is still an option. The installer still needs to know what is being installed with pymodbus[x,y,z] and what can only be installed with "--group x,y,z". I really do not see any advantages in confusing the installer. You could argue, that with the current method we should add "all-only-runtime", but that is quite over the top. |
|
If |
|
I agree that end users can install pyserial and simulator. But on some platforms you cannot install "all" because aiohttp and/or sphinx are not available. |
Do you mean that for dev only or also end users? |
|
endusers cannot install simulator and developers cannot install simulator and/or documentation |
|
The installation proces is not something invented overnight it has evolved over years and have been stable for the last couple of years.....so changing it is possible but you need to remember all the corner cases. |
Not sure what you mean by this. Do you mean that they shouldn't be able to do that or that they tried to with this PR and can't? |
|
Using pip would work like this now: Install required development tools in editable mode::
- pip install -e ".[development]"
+ pip install -e . --group dev
Install all (allows creation of documentation etc) in editable mode::
- pip install -e ".[all]"
+ pip install -e . --group allOr with uv: and end users can install the package |
|
So a developer, who cannot install simulator and documentation but wants pyserial, would need to do: pip install -e ".[pyserial] --group dev if correct the problem remains the same. you either need to drop --group or .[x,y,z] they cannot coexist without causing confusion. I use that combination quite often to solve serial problems. |
|
Ps. if dropping the current version there need to be a very good reason, because that is a bigger change, which affect all downstream fork and most likely a couple of app developers as well. alternatively you need to be able to continue using the current version and --group is an optional feature. |
|
You mentioned something about installing on an ESP board (I assume ESP32 or similar?). Are you installing pymodbus on the actual hardware? Like using micropython? Just curious about that. I could take a look at those downstream projects and help out if it's a problem. Alternatively we could support both (groups just reference the extras) so that either option will work. |
|
I think I know a potential issue with using groups. If someone uses |
|
requirements.txt is not a problem for pymodbus, but might be for downstream. You do not see the change as a problem, and I would agree if there was a real advantage in doing it....but right now there was only argument is "end-users could install dev tools without wanting it, however it is clearly stated in the documentation that all means all. If they do not read the documentation they do not know about all. problem with downstream projects is that you do not know if they are active....and some have not forced, but use submodule to avoid listíng pymodbus as a requirement (my interpretation). |
|
Yeah I guess we can just leave pyproject.toml as it is. Maybe it's worth adding a dev dependency group so that uv automatically installs |
It references the `development` extra so that it gets automatically installed with uv
30afbb1 to
86a8ce3
Compare
86a8ce3 to
25bcd1f
Compare
|
making uv install automatically sounds like a good idea. you also wanted to document uv usage in the readme. |
pyproject.tomlused to list development and documentation dependencies (pytest, zuban, sphinx, etc.) as optional dependencies. This means that it's possible to install development-only extras from PyPI.It doesn't really make sense to install dev/docs dependencies as an end user. Dependency groups (PEP 735) can be used instead to isolate those dependencies.
documentationanddevlopmenthave been moved into dependency groups. Additionallydevelopmentcan be renamed todevso that UV will automatically install them when working in the project (see: https://docs.astral.sh/uv/concepts/projects/dependencies/#development-dependencies)