Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added pop() method to ConfigTree and added KeyError to ConfigMissingException #120

Merged
merged 2 commits into from
Aug 16, 2017

Conversation

jjtk88
Copy link
Contributor

@jjtk88 jjtk88 commented Jul 14, 2017

Hi there,

I've added a pop() method to the ConfigTree class, and I'm 99% sure that I've done it correctly. Wanted to submit a pull request so that I don't have to keep patching locally. :-) I tried to cover all the edge cases in the test case, but I may have miss something.

Additionally, I added KeyError to the inheritance list for ConfigMissingException so that you can use it more like a dictionary, like:

c = ConfigTree()
..
try:
    v = c['my.key']
except KeyError:
    do_something()

The only thing I'm not entirely sure of is line 225-227. It works, although I wasn't sure if you guys had a more preferred way to split/join key names

My commit msg:

Added pop() method to ConfigTree that works like dict.pop(). Added test
cases as well

Also, appended KeyError to ConfigMissingException so that exceptions can
be caught upstream via normal dict() syntax

Added pop() method to ConfigTree that works like dict.pop().  Added test
cases as well

Also, appended KeyError to ConfigMissingException so that exceptions can
be caught upstream via normal dict() syntax
@coveralls
Copy link

coveralls commented Jul 14, 2017

Coverage Status

Coverage increased (+0.03%) to 97.956% when pulling f359910 on jjtk88:master into 30c9a51 on chimpler:master.

@darthbear
Copy link
Member

Looks good! Thanks @jjtk88 for the PR!

@darthbear darthbear merged commit f669ae8 into chimpler:master Aug 16, 2017
@darthbear
Copy link
Member

Pushed to pyhocon==0.3.36. Thanks again!

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.

None yet

3 participants