Skip to content

CLI submodel suppress. #587

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

Merged
merged 5 commits into from
Apr 14, 2025
Merged

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