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

Error when using settings class that has cli_parse_args=True in VSCode Notebook #476

Closed
Riezebos opened this issue Nov 12, 2024 · 4 comments · Fixed by #477
Closed

Error when using settings class that has cli_parse_args=True in VSCode Notebook #476

Riezebos opened this issue Nov 12, 2024 · 4 comments · Fixed by #477
Assignees

Comments

@Riezebos
Copy link

Problem

Some applications add cli args by default. E.g. VSCode:
image

I want to ignore this using:

model_config = SettingsConfigDict(cli_parse_args=True, cli_ignore_unknown_args=True)

but if I have a settings class with an attribute that starts with f it will still try to using this argument:

from pydantic_settings import BaseSettings, SettingsConfigDict

class MySettings(BaseSettings):
    model_config = SettingsConfigDict(cli_parse_args=True, cli_ignore_unknown_args=True)

    freeze: bool = False

MySettings()

Gives the following error:

ValidationError: 1 validation error for MySettings
freeze
  Input should be a valid boolean, unable to interpret input [type=bool_parsing, input_value='/home/simon/.local/share...234382f2c59f2a7cbf.json', input_type=str]
    For further information visit https://errors.pydantic.dev/2.9/v/bool_parsing

Root cause

This is because ArgumentParsers allows abbreviations by default:
https://docs.python.org/3/library/argparse.html#allow-abbrev

Proposed solution

Could you add an option to SettingsConfigDict to set allow_abrev=False in the _CliInternalArgParser?

Maybe this should even be the default, it can be quite confusing if the Settings class by default accepts any abbreviation of an argument as that argument.

Workarounds

In case anyone is running into the same, the simplest workaround is just not using attributes that start with f. Another solution is to check whether the program is running in ipython and if so set cli_parse_args to false.

Other error variant

Another fun example:

from pydantic_settings import BaseSettings, SettingsConfigDict

class MySettings(BaseSettings):
    model_config = SettingsConfigDict(cli_parse_args=True, cli_ignore_unknown_args=True)

    freeze: bool = False
    freeze_also: bool = False

MySettings()

Output:

An exception has occurred, use %tb to see the full traceback.

SystemExit: 2

[/home/simon/miniforge3/envs/asr_test/lib/python3.12/site-packages/IPython/core/interactiveshell.py:3585](https://vscode-remote+ssh-002dremote-002bwsl.vscode-resource.vscode-cdn.net/home/simon/miniforge3/envs/asr_test/lib/python3.12/site-packages/IPython/core/interactiveshell.py:3585): UserWarning: To exit: use 'exit', 'quit', or Ctrl-D.
  warn("To exit: use 'exit', 'quit', or Ctrl-D.", stacklevel=1)
usage: ipykernel_launcher.py [-h] [--freeze bool] [--freeze_also bool]
ipykernel_launcher.py: error: ambiguous option: --f=/home/simon/.local/share/jupyter/runtime/kernel-v3f2135e10095ca1cb1e8523234382f2c59f2a7cbf.json could match --freeze, --freeze_also
@hramezani
Copy link
Member

@kschwab Could you please take a look?

@kschwab
Copy link
Contributor

kschwab commented Nov 12, 2024

@Riezebos thanks for the find! I wasn't aware of the allow_abbrev=True default behavior on argparse. I agree it is confusing if not intentionally set. I'm going to set it to False without adding a config flag, mostly because I don't believe the behavior is consistent with the rest of the pydantic settings sources.

However, I'm not sure it will solve the issue above. e.g., error: ambiguous option: --f=/home/simon/.... does not look like it should be a CLI abbreviation for the MySettings models. Could you verify locally?
-- Nevermind, I was able to reproduce it locally and validate it was indeed caused by abbreviations 👍🏾

I've opened #477 with abbreviations disabled.

@hramezani
Copy link
Member

Thanks @kschwab for the quick fix.

@Riezebos Could you please confirm the fix?

@Riezebos
Copy link
Author

Yes, works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants