ConfigArgParse icon indicating copy to clipboard operation
ConfigArgParse copied to clipboard

Writing out configuration does not properly handle option with nargs greater than 1

Open clydeclements opened this issue 7 years ago • 3 comments

When requesting that a configuration file be saved, an option with multiple arguments is not properly preserved. For example, consider this script called test.py:

import configargparse

p = configargparse.ArgParser(
    config_file_parser_class=configargparse.YAMLConfigFileParser,
    args_for_writing_out_config_file=['-w', '--write-out-config-file']
)
p.add('-c', '--config', is_config_file=True, help='config file path')
p.add('-i', '--indexes', type=int, metavar=('x', 'y'), nargs=2)

options = p.parse_args()

print(options)
print("----------")
print(p.format_help())
print("----------")
print(p.format_values())

Calling this script as follows:

python test.py --indexes 2 4 -w save_config.yaml

produces a save_config.yaml file with contents:

indexes: '2'

The configuration file is not valid: the second argument value, 4, is lost. The previous version of ConfigArgParse, version 0.11, did not suffer from this problem. With that version, save_config.yaml contains:

indexes:
- 2
- 4
write-out-config-file: save_config.yaml

A quick glance through the code history and It looks to me like commit 8ff6cca broke this with its change in function get_items_for_config_file_output. Thoughts?

clydeclements avatar Aug 09 '17 18:08 clydeclements

I did not review that PR carefully enough. If you have time to look in to this - do you see a good way to fix that elif: https://github.com/bw2/ConfigArgParse/commit/8ff6ccad8af1024d9e694f40e7dd69e9cdcc188b#diff-278b05a12fc2a696eef108413d532841R625
?

bw2 avatar Dec 26 '17 19:12 bw2

+1, any progress in this matter?

mhoff avatar Jul 05 '18 12:07 mhoff

I've taken a closer look at this and to me, the changes in commit 8ff6cca were unnecessary. The original poster reported an issue with config file writes not preserving values of custom type. I think no change is necessary to configargparse.py for this case. Instead, define a __str__ method for the custom type, which will be called when the object is serialized for writing out to the config file.

I've undone the small change to configargparse.py in commit 8ff6cca and I've added a __str__ method to the CustomClass in the test file so that all tests still pass. Changes can be seen in the pull request #130.

clydeclements avatar Aug 10 '18 00:08 clydeclements