Skip to content

Ensure auto_env_var_prefix converts dash to underscore like ArgParse#29

Merged
bw2 merged 1 commit intobw2:masterfrom
helgi:underscore_env
Nov 9, 2015
Merged

Ensure auto_env_var_prefix converts dash to underscore like ArgParse#29
bw2 merged 1 commit intobw2:masterfrom
helgi:underscore_env

Conversation

@helgi
Copy link
Contributor

@helgi helgi commented Nov 5, 2015

From https://docs.python.org/dev/library/argparse.html#dest: Any internal - characters will be converted to _ characters to make sure the string is a valid attribute name

Also bumped the version to 0.10.0 to be more semvar since 0.9.4 was really feature addition.

@bw2
Copy link
Owner

bw2 commented Nov 9, 2015

Hi @helgi I'm hesitating to merge this because there are many special characters that can't be included in auto_env_var_prefix (eg. '=', etc.), so I'd rather just leave it to the user to provide a sensible auto_env_var_prefix value instead of trying to fix specific kinds of bad characters like '-' .

@helgi
Copy link
Contributor Author

helgi commented Nov 9, 2015

This actually has nothing to do with the prefix it self but rather the arguments themselves. It is transforming --foo-bar with a HELLO_ prefix to HELLO_FOO_BAR instead of being saved as HELLO_FOO-BAR.

Right now the code in master straight up takes the config name, no transformation.

Replacing - with _ follows the argparse convention (linked to above) and also makes it possible for people to set ENV Vars that are separated with - in the config args.

export HELLO_FOO-BAR=100
-bash: export: `HELLO_FOO-BAR=100': not a valid identifier

Shell doesn't like the - it seems.

From https://docs.python.org/dev/library/argparse.html#dest: Any internal - characters will be converted to _ characters to make sure the string is a valid attribute name
@bw2
Copy link
Owner

bw2 commented Nov 9, 2015

Oh, I see. My mistake.
Thanks for the pull request!

bw2 added a commit that referenced this pull request Nov 9, 2015
Ensure auto_env_var_prefix converts dash to underscore like ArgParse
@bw2 bw2 merged commit 7070553 into bw2:master Nov 9, 2015
@helgi
Copy link
Contributor Author

helgi commented Nov 9, 2015

Thanks for merging! I can see why it would be confusing given people can add in their own prefix ending and I didn't specifically mention values only :-)

For a few minutes I did think about potentially just auto adding _ between prefix and the value as well but some people might not want any _ so I left that alone.

@torrance
Copy link

torrance commented Dec 7, 2016

Does it make sense to do the same for all configuration sources, except the command line?

For example, I would expect p.add_arguments('-c', '--config-file') to have a corresponding yaml key of 'config_file'.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants