djay0529 / mdanalysis

Automatically exported from code.google.com/p/mdanalysis
0 stars 0 forks source link

Timestep copy not working #164

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The __deepcopy__ method in base.Timestep returns a base.Timestep rather than an 
appropriate format Timestep.

The  branch below fixes this, as well as fixing a couple places where Timesteps 
weren't copying perfectly.  There is also unit testing for every format 
Timestep making a copy operation.

https://code.google.com/r/richardjgowers-topologylists/source/list?name=TScopy

Original issue reported on code.google.com by richardjgowers on 2 Feb 2014 at 5:59

GoogleCodeExporter commented 9 years ago

Original comment by orbeckst on 2 Feb 2014 at 11:25

GoogleCodeExporter commented 9 years ago
Richard,

a bunch of tests failed for me when I merged your branch into my latest develop:

FAILED
 test_coordinates:Timestep_Copy_TRJ  -- no status attrib
 test_coordinates:Timestep_Copy_TRR  -- no status attrib
 test_coordinates:Timestep_Copy_TRZ  -- no _velocities attrib

Can you please look into this?

Can you please follow the test naming convention and start all testcase classes 
that are to be run with "Test". If you want to define a mixin class start with 
an underscore. However, in your case it's ok to have TestTimestep_Copy just as 
it is as it checks the base class (which happens to be also DCD).

Don't forget to update CHANGELOG. The commit message can contain "(fixes Issue 
164)" and then this issue will be automatically closed. Feel free to rewrite 
your history to keep everything in two patches (one for the test cases and one 
for the code) and do "push -f", I can do a forced pull from your branch. If you 
rewrite your history, just mention it in the message in the issue report.

Btw, good approach to write the tests FIRST!!!

Oli

Original comment by orbeckst on 4 Feb 2014 at 12:39

GoogleCodeExporter commented 9 years ago

Original comment by orbeckst on 4 Feb 2014 at 12:39

GoogleCodeExporter commented 9 years ago
Hmm, that's odd.  Sounds like git didn't give you the fixes but only the tests.

I rewrote the branch history to name the tests correctly.  The TScopy branch 
should now have 2 commits, unit tests then fixes.

Hopefully it works this time

Original comment by richardjgowers on 4 Feb 2014 at 12:17

GoogleCodeExporter commented 9 years ago
This issue was closed by revision 2d5a068c8747.

Original comment by richardjgowers on 4 Feb 2014 at 7:06

GoogleCodeExporter commented 9 years ago
Thanks – in fact there was an issue with the way I ran the UnitTests. 
Apparently, a developer installation does not work when running the tests like

  nosetests -v MDAnalysisTests.test_coordinates:TestTimestep_Copy_TRR

Your changes are now in develop.

Original comment by orbeckst on 4 Feb 2014 at 7:07

GoogleCodeExporter commented 9 years ago
This issue was closed by revision 2d5a068c8747.

Original comment by richardjgowers on 8 Feb 2014 at 2:17