-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: Add default-group-optionality configuration option #10623
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
base: main
Are you sure you want to change the base?
feat: Add default-group-optionality configuration option #10623
Conversation
This commit implements a new configuration option `default-group-optionality` that allows users to treat all dependency groups as optional by default. When set to `true`, Poetry will only install the implicit `main` group by default, and other groups must be explicitly requested using `--with` or `--only` flags. This eliminates the need to mark each group as `optional = true` in pyproject.toml for projects with many optional dependency groups. Changes: - Add `default-group-optionality` boolean config option (default: false) - Modify GroupCommand.non_optional_groups to respect the configuration - Add comprehensive tests for the new functionality - Update documentation with usage examples Fixes python-poetry#10550
Reviewer's GuideIntroduces a new default-group-optionality boolean setting that makes all dependency groups optional by default, updates GroupCommand to respect this option, extends configuration handling and documentation, and adds tests covering the new behavior. Class diagram for updated configuration handling (Config class)classDiagram
class Config {
+bool default-group-optionality = False
+__init__(use_environment: bool)
+_get_normalizer(name: str)
}
Class diagram for updated GroupCommand logicclassDiagram
class GroupCommand {
+non_optional_groups: set[str]
}
GroupCommand --> Config : uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- The non_optional_groups override currently treats every group as optional when default-group-optionality is true—consider preserving any explicit optional = false declarations in pyproject.toml so users can still force non-optional groups.
- Per the existing TODO, it would be cleaner to move the default-group-optionality logic out of the console command and into poetry-core to centralize dependency-group handling.
- The documentation examples don’t specify --local or --global when setting the config—consider clarifying which scope is used in those examples for better user guidance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The non_optional_groups override currently treats every group as optional when default-group-optionality is true—consider preserving any explicit optional = false declarations in pyproject.toml so users can still force non-optional groups.
- Per the existing TODO, it would be cleaner to move the default-group-optionality logic out of the console command and into poetry-core to centralize dependency-group handling.
- The documentation examples don’t specify --local or --global when setting the config—consider clarifying which scope is used in those examples for better user guidance.
## Individual Comments
### Comment 1
<location> `tests/console/commands/test_default_group_optionality.py:112-136` </location>
<code_context>
+ assert installer_groups == {MAIN_GROUP}
+
+
+def test_with_default_group_optionality_and_with_option(
+ install_tester: CommandTester, mocker: MockerFixture
+) -> None:
+ """
+ With default-group-optionality enabled, --with can explicitly include groups.
+ """
+ # Enable the configuration
+ install_tester.command.poetry.config.merge({"default-group-optionality": True})
+
+ mocker.patch.object(install_tester.command.installer, "run", return_value=0)
+ mocker.patch(
+ "poetry.masonry.builders.editable.EditableBuilder",
+ side_effect=Exception("Should not be called"),
+ )
+
+ status_code = install_tester.execute("--no-root --with test,dev")
+ assert status_code == 0
+
+ # Should install main, test, and dev groups
+ installer_groups = set(install_tester.command.installer._groups or [])
+ assert installer_groups == {MAIN_GROUP, "test", "dev"}
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Missing test for invalid group names in --with option.
Add a test case using --with with a non-existent group name to confirm the CLI handles invalid input correctly, such as raising an error or ignoring unknown groups.
```suggestion
+
+
def test_with_default_group_optionality_and_with_option(
install_tester: CommandTester, mocker: MockerFixture
) -> None:
"""
With default-group-optionality enabled, --with can explicitly include groups.
"""
# Enable the configuration
install_tester.command.poetry.config.merge({"default-group-optionality": True})
mocker.patch.object(install_tester.command.installer, "run", return_value=0)
mocker.patch(
"poetry.masonry.builders.editable.EditableBuilder",
side_effect=Exception("Should not be called"),
)
status_code = install_tester.execute("--no-root --with test,dev")
assert status_code == 0
# Should install main, test, and dev groups
installer_groups = set(install_tester.command.installer._groups or [])
assert installer_groups == {MAIN_GROUP, "test", "dev"}
def test_with_default_group_optionality_and_invalid_group(
install_tester: "CommandTester", mocker: "MockerFixture"
) -> None:
"""
Using --with with a non-existent group name should result in an error.
"""
# Enable the configuration
install_tester.command.poetry.config.merge({"default-group-optionality": True})
mocker.patch.object(install_tester.command.installer, "run", return_value=0)
mocker.patch(
"poetry.masonry.builders.editable.EditableBuilder",
side_effect=Exception("Should not be called"),
)
# Use a non-existent group name
status_code = install_tester.execute("--no-root --with test,nonexistentgroup")
# Should return a non-zero status code indicating error
assert status_code != 0
# Optionally, check for error message about unknown group
output = install_tester.io.fetch_error()
assert "nonexistentgroup" in output
assert "Unknown dependency group" in output or "does not exist" in output
```
</issue_to_address>
### Comment 2
<location> `tests/console/commands/test_default_group_optionality.py:136-158` </location>
<code_context>
+ assert installer_groups == {MAIN_GROUP, "test", "dev"}
+
+
+def test_with_default_group_optionality_and_only_option(
+ install_tester: CommandTester, mocker: MockerFixture
+) -> None:
+ """
+ With default-group-optionality enabled, --only still works as expected.
+ """
+ # Enable the configuration
+ install_tester.command.poetry.config.merge({"default-group-optionality": True})
+
+ mocker.patch.object(install_tester.command.installer, "run", return_value=0)
+ mocker.patch(
+ "poetry.masonry.builders.editable.EditableBuilder",
+ side_effect=Exception("Should not be called"),
+ )
+
+ status_code = install_tester.execute("--no-root --only test")
+ assert status_code == 0
+
+ # Should only install test group
+ installer_groups = set(install_tester.command.installer._groups or [])
+ assert installer_groups == {"test"}
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Missing test for combining --only and --with flags.
Please add a test case that uses both --only and --with flags together with default-group-optionality enabled to confirm their combined behavior.
```suggestion
def test_with_default_group_optionality_and_only_and_with_flags(
install_tester: "CommandTester", mocker: "MockerFixture"
) -> None:
"""
With default-group-optionality enabled, using --only and --with together
should only install the group specified by --only.
"""
# Enable the configuration
install_tester.command.poetry.config.merge({"default-group-optionality": True})
mocker.patch.object(install_tester.command.installer, "run", return_value=0)
mocker.patch(
"poetry.masonry.builders.editable.EditableBuilder",
side_effect=Exception("Should not be called"),
)
status_code = install_tester.execute("--no-root --only test --with dev")
assert status_code == 0
# Should only install test group, --with should not add dev when --only is present
installer_groups = set(install_tester.command.installer._groups or [])
assert installer_groups == {"test"}
```
</issue_to_address>
### Comment 3
<location> `tests/console/commands/test_default_group_optionality.py:169-178` </location>
<code_context>
+ assert "false" in config_tester.io.fetch_output().strip().lower()
+
+
+def test_config_set_default_group_optionality(config_tester: CommandTester) -> None:
+ """
+ Test setting the default-group-optionality configuration value.
+ """
+ config_tester.execute("--local default-group-optionality true")
+ assert config_tester.status_code == 0
+
+ config_tester.io.clear_output()
+ config_tester.execute("--local default-group-optionality")
+ assert config_tester.status_code == 0
+ assert "true" in config_tester.io.fetch_output().strip().lower()
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Missing test for invalid config value types.
Add a test that sets default-group-optionality to an invalid type (e.g., a string or integer) to verify that the system rejects it and returns an appropriate error message.
Suggested implementation:
```python
def test_config_set_default_group_optionality_invalid_type(config_tester: CommandTester) -> None:
"""
Test setting the default-group-optionality configuration value to an invalid type.
"""
# Try setting to a string that is not a boolean
config_tester.execute("--local default-group-optionality notabool")
assert config_tester.status_code != 0
assert "invalid" in config_tester.io.fetch_error().strip().lower() or "error" in config_tester.io.fetch_error().strip().lower()
config_tester.io.clear_error()
# Try setting to an integer
config_tester.execute("--local default-group-optionality 123")
assert config_tester.status_code != 0
assert "invalid" in config_tester.io.fetch_error().strip().lower() or "error" in config_tester.io.fetch_error().strip().lower()
from typing import TYPE_CHECKING
import pytest
from poetry.core.packages.dependency_group import MAIN_GROUP
```
If your test framework or CLI does not use `fetch_error()` for error output, replace it with the appropriate method to fetch error messages. Also, ensure that the error message contains "invalid" or "error"—adjust the assertion string as needed to match your actual error output.
</issue_to_address>
### Comment 4
<location> `tests/console/commands/test_default_group_optionality.py:15` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Don't import test modules. ([`dont-import-test-modules`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/dont-import-test-modules))
<details><summary>Explanation</summary>Don't import test modules.
Tests should be self-contained and don't depend on each other.
If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.
</details>
</issue_to_address>
### Comment 5
<location> `tests/console/commands/test_default_group_optionality.py:16` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Don't import test modules. ([`dont-import-test-modules`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/dont-import-test-modules))
<details><summary>Explanation</summary>Don't import test modules.
Tests should be self-contained and don't depend on each other.
If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.
</details>
</issue_to_address>
### Comment 6
<location> `src/poetry/console/commands/group_command.py:48-50` </location>
<code_context>
@property
def non_optional_groups(self) -> set[str]:
# TODO: this should move into poetry-core
default_optional = self.poetry.config.get("default-group-optionality", False)
if default_optional:
# When default-group-optionality is True, all groups are optional
return set()
return {
group.name
for group in self.poetry.package._dependency_groups.values()
if not group.is_optional()
}
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if default_optional := self.poetry.config.get(
"default-group-optionality", False
):
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # TODO: this should move into poetry-core | ||
| default_optional = self.poetry.config.get("default-group-optionality", False) | ||
| if default_optional: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Use named expression to simplify assignment and conditional (use-named-expression)
| # TODO: this should move into poetry-core | |
| default_optional = self.poetry.config.get("default-group-optionality", False) | |
| if default_optional: | |
| if default_optional := self.poetry.config.get( | |
| "default-group-optionality", False | |
| ): |
|
Hey! I saw issue #10550 and thought this would be useful. Let me know if you'd like any changes! 😊 |
This commit implements a new configuration option
default-group-optionalitythat allows users to treat all dependency groups as optional by default.When set to
true, Poetry will only install the implicitmaingroup by default, and other groups must be explicitly requested using--withor--onlyflags. This eliminates the need to mark each group asoptional = truein pyproject.toml for projects with many optional dependency groups.Changes:
default-group-optionalityboolean config option (default: false)Fixes #10550
Pull Request Check List
Resolves: #issue-number-here
Summary by Sourcery
Add a new
default-group-optionalityconfiguration option to allow treating all dependency groups as optional by default and update related command logic, documentation, and tests.New Features:
default-group-optionalityboolean config option to toggle default optionality of dependency groupsEnhancements:
default-group-optionalitysetting when determining which groups to installDocumentation:
default-group-optionalityoption in the configuration guide with usage examplesTests:
default-group-optionalitybehavior, including install scenarios and config commands