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

analysis.rms.RMSD broken #897

Closed
3 tasks done
orbeckst opened this issue Jul 8, 2016 · 3 comments · Fixed by #889
Closed
3 tasks done

analysis.rms.RMSD broken #897

orbeckst opened this issue Jul 8, 2016 · 3 comments · Fixed by #889

Comments

@orbeckst
Copy link
Member

orbeckst commented Jul 8, 2016

Expected behaviour

I should be able to set a RMSD analysis with MDAnalysis.analysis.rms.RMSD

Actual behaviour

Raises NameError.

Analysis

It seems that recently _process_selection() was renamed to process_selection() in 46f915b --- @jdetle seems to have forgotten to also rename references.

Even worse: There are no unit tests for the RMSD class! There's a test TestRMSD but it does not test the RMSD class but the rmsd() function. We should

  • fix rms.RMSD
  • rename TestRMSD to Testrmsdfunc (or just Testrms --- it all looks ugly?)
  • add a test TestRMSD for rms.RMSD

Stack Trace

---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-6-be62615c3dd7> in <module>()
----> 1 R = RMSD(u, select="protein and name CA")

/nfs/homes2/oliver/Library/python/mdanalysis/package/MDAnalysis/analysis/rms.pyc in __init__(self, traj, reference, select, groupselections, filename, mass_weighted, tol_mass, ref_frame)
    339         else:
    340             self.reference = reference
--> 341         self.select = _process_selection(select)
    342         if groupselections is not None:
    343             self.groupselections = [_process_selection(s) for s in groupselections]

NameError: global name '_process_selection' is not defined

Code to reproduce the behaviour

import MDAnalysis as mda
from MDAnalysis.tests.datafiles import PSF, DCD
from MDAnalysis.analysis.rms import RMSD
u = mda.Universe(PSF, DCD)
R = RMSD(u, select="protein and name CA")

Currently version of MDAnalysis:

(run python -c "import MDAnalysis as mda; print(mda.__version__)")
0.15.1-dev0

@orbeckst
Copy link
Member Author

orbeckst commented Jul 8, 2016

(By the way, not having tests for rms.RMSD is probably my own fault...)

@jdetle
Copy link
Contributor

jdetle commented Jul 8, 2016

I broke it, someone else didn't cover the code, let's just call it a draw...

@orbeckst
Copy link
Member Author

orbeckst commented Jul 8, 2016

Sounds fair ;-)

@jdetle jdetle mentioned this issue Jul 8, 2016
11 tasks
orbeckst pushed a commit that referenced this issue Jul 10, 2016
Fixed (temporarily) broken analysis.rms.RMSD and added missing tests

- fixes #897 
- full testing of the RMSD class

Updated CHANGELOG

* Added tests for RMSD, added a testfile.
Changed RMSD class to save rmsd array as numpy file instead of text.

* Tested saving

* Test group selections
Reverted back to save.
Updated test coverage for exceptions.
Found some errors with logging requiring a message.
Reverted inclusion of unnecessary RMSD array.
Fixed a small error and combined save and mass_weighted test.
No Data Error.
Covered RMSD.save().

* Fixed error messages

* Asserted everywhere, fixed error messages.
abiedermann pushed a commit to abiedermann/mdanalysis that referenced this issue Oct 26, 2016
Fixed (temporarily) broken analysis.rms.RMSD and added missing tests

- fixes MDAnalysis#897 
- full testing of the RMSD class

Updated CHANGELOG

* Added tests for RMSD, added a testfile.
Changed RMSD class to save rmsd array as numpy file instead of text.

* Tested saving

* Test group selections
Reverted back to save.
Updated test coverage for exceptions.
Found some errors with logging requiring a message.
Reverted inclusion of unnecessary RMSD array.
Fixed a small error and combined save and mass_weighted test.
No Data Error.
Covered RMSD.save().

* Fixed error messages

* Asserted everywhere, fixed error messages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants