Failure to check choice matches for options when using a configuration yaml or json file.

[ArenaGrenade]

🐛 Bug report

Using a config file (yaml or json) skips the choice validation step for any other argument.

To reproduce

import jsonargparse
import yaml

parser = jsonargparse.ArgumentParser()
parser.add_argument('--arg1', type=str, choices=["choice1", "choice2"])
parser.add_argument("--config", action=jsonargparse.ActionConfigFile)
config = yaml.safe_dump(
    {
        "arg1": "i_wont_choose_from_choices",
    }
)
result = parser.parse_args([f"--config={config}"])
print("Skips choice validation and wrong config is parsed:")
print(parser.dump(result))

print("\n----------The above one parses wrongly. The expected behaviour follows---------------------\n\n")

print("But pass the arg1 as a argument override and it immediately fails. As its supposed to - the tests make sure of this.")
result = parser.parse_args([f"--arg1=i_definitely_wont_choose_from_choices"])
print(parser.dump(result))

Expected behavior

The same script above shows the expected behavior as well. But what should happen is parser should throw an error stating that the given argument is not valid as it is not within the set of options given.

Environment

• jsonargparse version (e.g., 4.8.0): 4.25.0
• Python version (e.g., 3.9): 3.8.18
• How jsonargparse was installed (e.g. pip install jsonargparse[all]): as in example
• OS (e.g., Linux): Ubuntu 22.04 LTS

My initial suspicions are that this has to do with how config file arguments are parsed.
This line (calling a function from argparse which further down the line calls the choice validation function in argparse):

jsonargparse/jsonargparse/_core.py

Line 259 in 5fc8357

namespace, args = self._parse_known_args(args, namespace)

is on a high level responsible for choice validation. But, in the current code, the config argument parsing doesn't fill out the other fields before the validation work is handed over to argparse. So whatever arguments come from the config file are not at all choice checked. The current code does have choice checking but that is only for subcommands and not for '--arg' (not sure what to call them but the ones with dashes or just arguments). Hence, any choice arguments provided via a config file go under the radar and can potentially wreak havoc down the line.

[mauvilsa]

The "To reproduce" section above is empty. Was it lost by mistake?

[ArenaGrenade]

Yeah, sorry. Ended up hidden in a comment. Fixed now! PTAL

[mauvilsa]

Thank you for reporting! This is fixed in pull request #405.

[ArenaGrenade]

Hey! Thanks for the fix, it works perfectly now.

Wondering if this is just a quick fix and will be investigated later, or is the best way to go about it? Given underlying argparse parser already handles choice validation and a bunch of other things with the referenced line of code in my issue, won't there be issues with the other things that argparse parser does? Haven't exactly checked on the other functionality of that line of code, so not sure about the exact problems that might arise.

[mauvilsa]

The thing is that jsonargparse extends argparse in ways it was never designed to allow. There several other are things reimplemented in jsonargparse that already existed in argparse. But that is the price to pay to have many more advanced features and still be argparse-compatible up to a few intended deviations. Note #234. Hopefully one day I will add running the argparse unit tests to be more sure about its compatibility. But for instance this issue. Config files are not supported by argparse, so there is no way that argparse code should be able to handle it.

The fix might not be ideal, but certainly good enough. The most important part is the added unit test to make sure there are no regressions. You are free dig into the code to see how to refactor and improve the code for the better. Also you could contribute by writing unit tests for more cases and maybe identify other unknown bugs.