Skip to content

Conversation

@fkloft
Copy link
Contributor

@fkloft fkloft commented May 2, 2018

Currently, metadata items are assigned to a dict that is stored as an attribute of the argument parser. The proper solution would be to assign the dict to the argument namespace, which is also the way the Python docs suggest.

Also, the generation of default values directly on the argument namespace is suppressed to not litter it with None values.

@coveralls
Copy link

coveralls commented May 2, 2018

Coverage Status

Coverage decreased (-82.04%) to 0.0% when pulling 55071cf on fkloft:argparser into db0e3dc on LibraryOfCongress:master.

@acdha
Copy link
Member

acdha commented Sep 17, 2018

It looks like most of the changed lines are whitespace-related. Can you run your branch through Black first?

Felix Kloft added 3 commits September 17, 2018 19:27
Python docs recommend to add properties to namespace, this allows for parsers to be reused.
attribute wouldn't be set without any metadata args. also removes the need to check for AttributeError
@fkloft
Copy link
Contributor Author

fkloft commented Sep 17, 2018

Sorry, this was an old PR. When I tried to merge the current master, I accidentally reverted some whitespace related changes from master. I rebased my commits onto master which fixed the whitespace issues.

@acdha acdha merged commit ab226af into LibraryOfCongress:master Sep 17, 2018
@fkloft fkloft deleted the argparser branch March 14, 2019 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants