Skip to content

Conversation

@kschwab
Copy link
Contributor

@kschwab kschwab commented Apr 13, 2025

Fixes #586

@hramezani
Copy link
Member

Thanks @kschwab. I resolved the conflicts

@hramezani hramezani merged commit ed7fd42 into pydantic:main Apr 14, 2025
18 checks passed
@Geo5
Copy link

Geo5 commented Apr 19, 2025

Hey, thanks for working on this already!
I think the fix is not quite enough, because it fails to do what it is supposed to do if you add a description to the SubModel (either via Field(description=...) or as class docstring and setting cli_use_class_docs_for_groups=True).

I think the fix would be to also set CLI_SUPPRESS to the subparser description field here:

model_group_kwargs['description'] = field_info.description

e.g.:

        is_model_suppressed = self._is_field_suppressed(field_info) or is_model_suppressed
        model_group_kwargs['description'] = CLI_SUPPRESS if is_model_suppressed else field_info.description

like doing it a bit later in this function when adding the json argument:

is_model_suppressed = self._is_field_suppressed(field_info) or is_model_suppressed

I am not sure if this is documented behavior by argparse, but i looked at the source and this is what it does. ¹


¹ See here:
https://github.com/python/cpython/blob/main/Lib/argparse.py#L252
Which gets called here:
https://github.com/python/cpython/blob/main/Lib/argparse.py#L2622
With the description field, which leads to the section returning no help string here:
https://github.com/python/cpython/blob/main/Lib/argparse.py#L222
if all argument texts are also empty (which they are with the above pull request, as all SubModel fields behave as if they had CLI_SUPPRESS explicitly)

@Geo5
Copy link

Geo5 commented Apr 19, 2025

The test can be made to fail with this test changes:

diff --git a/tests/test_source_cli.py b/tests/test_source_cli.py
index 6c523ff..6eb7421 100644
--- a/tests/test_source_cli.py
+++ b/tests/test_source_cli.py
@@ -2153,6 +2153,8 @@ def test_cli_app_exceptions():
 
 def test_cli_suppress(capsys, monkeypatch):
     class DeepHiddenSubModel(BaseModel):
+        """DeepHiddenSubModel class docstring"""
+
         deep_hidden_a: int
         deep_hidden_b: int
 
@@ -2166,10 +2168,10 @@ def test_cli_suppress(capsys, monkeypatch):
         visible_b: int
         deep_hidden_obj: CliSuppress[DeepHiddenSubModel]
 
-    class Settings(BaseSettings, cli_parse_args=True):
+    class Settings(BaseSettings, cli_parse_args=True, cli_use_class_docs_for_groups=True):
         field_a: CliSuppress[int] = 0
         field_b: str = Field(default=1, description=CLI_SUPPRESS)
-        hidden_obj: CliSuppress[HiddenSubModel]
+        hidden_obj: CliSuppress[HiddenSubModel] = Field(description='hidden_obj description')
         visible_obj: SubModel

@kschwab
Copy link
Contributor Author

kschwab commented Apr 21, 2025

Thanks for the catch @Geo5! I opened #604 for resolution.

@kschwab kschwab deleted the cli_submodel_suppress branch April 21, 2025 12:54
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 this pull request may close these issues.

CliSuppress does not work on sub-models

3 participants