{{ message }}
Repository navigation
Fix Config.to_args for list values and False booleans - #1806
Open
MohammadHijjawi97 wants to merge 1 commit into
Open
MohammadHijjawi97 wants to merge 1 commit into
MohammadHijjawi97 wants to merge 1 commit into
Conversation
`Config.to_args` documents support for bool, int, str, float and lists of them, but: - any list value always failed, because the element-type assertion was applied to the list itself instead of its items; - a `False` bool fell through to the int branch (bool is a subclass of int) and produced `--flag False`, which argparse rejects for a store_true flag. Check the type of each list element and pass the elements as separate arguments so they work with `nargs`, and omit the flag for `False`.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Config.to_argsdocuments that config values may be bool, int, str, float or a list of them, but two of these cases are broken:isinstance(v, (int, str, float, bool))), so any list value raisesAssertionError: Element type in list is expected to be either int,str,float, but got type 0.Falsebooleans produce invalid args.boolis a subclass ofint, soFalsefalls through to the int branch and emits--flag False. argparse rejects that for astore_trueflag (error: unrecognized arguments: False).Reproduction:
Fix:
nargs;False.Test: added
test_to_args_with_list_and_booltotests/utils/test_config.py; it fails on master and passes with this change. flake8, isort and yapf are clean on the changed files.