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

lib.util.Namespace raises the expected exception #1169

Merged
merged 4 commits into from
Jan 19, 2017

Conversation

jbarnoud
Copy link
Contributor

lib.util.Namespace allows to query a namespace by index or by
attribute. It allows to use both the following syntaxes to access the
same element:

ns = lib.util.Namespace()
ns['key'] = 'plop'

ns['key']  # Access by index
ns.key  # Access by attribute

The class is used to permit some flexibility when accessing auxiliary
data attached to trajectories.

Prior to this commit, accessing a non-existing item from a namespace by
attribute would raise a KeyError while an AttributeError should be
raise. This breaks functions that test the features of an object by
looking for some attributes, and are therefore capturing AttributeError
as a sign the attribute is missing.

In particular, raising the wrong exception confused
numpy.lib.type_check.iscomplexobj in numpy 1.12.0. This confusion lead
to assert_equal to miss behave in that version of numpy, therefore
breaking the tests for MDAnalysis when upgrading from python 1.11.3 to
1.12.0.

This commit makes sure that the expected exception is raised when
accessing an non-existing item by attribute in a Namespace object.

Adresses part of #1166

PR Checklist

  • Tests?
  • [ ] Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

`lib.util.Namespace` allows to query a namespace by index or by
attribute. It allows to use both the following syntaxes to access the
same element:

    ns = lib.util.Namespace()
    ns['key'] = 'plop'

    ns['key']  # Access by index
    ns.key  # Access by attribute

The class is used to permit some flexibility when accessing auxiliary
data attached to trajectories.

Prior to this commit, accessing a non-existing item from a namespace by
attribute would raise a KeyError while an AttributeError should be
raise. This breaks functions that test the features of an object by
looking for some attributes, and are therefore capturing AttributeError
as a sign the attribute is missing.

In particular, raising the wrong exception confused
`numpy.lib.type_check.iscomplexobj` in numpy 1.12.0. This confusion lead
to `assert_equal` to miss behave in that version of numpy, therefore
breaking the tests for MDAnalysis when upgrading from python 1.11.3 to
1.12.0.

This commit makes sure that the expected exception is raised when
accessing an non-existing item by attribute in a Namespace object.
@@ -91,6 +91,8 @@ Fixes
* Fixed selections using operators backwards ('prop 10 > mass') and sensitivity
about whitespace around these (PR #1156 Issue #1011 #1009)
* Fixed PSA analysis is now using AlignTraj
* The namespace used for auxiliary data now raises an AttributeError instead
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't NameSpace new in 0.16? If so, we don't have to document any fixes to it, (because CHANGELOG is for release to relrease changes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. I always forget what gets introduced when.

jbarnoud and others added 2 commits January 17, 2017 12:46
Added tests for lib.util.Namespace

Moved test_util to lib/test_util
@richardjgowers richardjgowers self-assigned this Jan 18, 2017
Added tests for lib.util.Namespace

Moved test_util to lib/test_util
@kain88-de
Copy link
Member

@richardjgowers is this OK for you?

@richardjgowers richardjgowers merged commit 282e57f into MDAnalysis:develop Jan 19, 2017
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