Skip to content

define __contains__ in EasyConfig class#1155

Merged
boegel merged 5 commits intoeasybuilders:developfrom
boegel:EasyConfig_contains
Feb 4, 2015
Merged

define __contains__ in EasyConfig class#1155
boegel merged 5 commits intoeasybuilders:developfrom
boegel:EasyConfig_contains

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Feb 3, 2015

If no __contains__ is defined, Python falls back to using __getitem__ for key in easyconfig_instance type of expressions (see https://docs.python.org/2/reference/datamodel.html#object.__contains__)

this patch fixes the issue that this fails hard if such an expression is used with a non-existing key

it also makes sure that a decent error message is shown whenever an undefined easyconfig parameter is used (get or set)

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Feb 3, 2015

@wpoely86: please review?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe print value too?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, there is no value, since the provided key doesn't exist...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, this is setting, my bad

@hpcugentbot
Copy link
Copy Markdown

Test FAILed.

@hpcugentbot
Copy link
Copy Markdown

Test PASSed.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Feb 4, 2015

going in, thanks for the review @wpoely86!

boegel added a commit that referenced this pull request Feb 4, 2015
define __contains__ in EasyConfig class
@boegel boegel merged commit 2d47fae into easybuilders:develop Feb 4, 2015
@boegel boegel deleted the EasyConfig_contains branch February 4, 2015 20:29
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