DCD XDR pickle tests

[yuxuanzhuang]

Fixes #2878

Changes made in this Pull Request:

• add inclusive tests for pickle low-level dcd and xdr files
• fix the bug that was introduced in Serialize FileIO and TextIOWrapper and Universe #2723
• may further backport in backport fix for #2878 (pickle DCD and XDR readers at correct frame) #2908
  PR Checklist

---

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

[pep8speaks]

Hello @yuxuanzhuang! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-08-16 14:55:01 UTC

[codecov]

Codecov Report

Merging #2911 into develop will decrease coverage by 0.01%.
The diff coverage is 64.70%.

@@             Coverage Diff             @@
##           develop    #2911      +/-   ##
===========================================
- Coverage    92.95%   92.94%   -0.02%     
===========================================
  Files          187      187              
  Lines        24579    24591      +12     
  Branches      3185     3185              
===========================================
+ Hits         22847    22855       +8     
- Misses        1686     1690       +4     
  Partials        46       46              

Impacted Files	Coverage Δ
package/MDAnalysis/lib/formats/libmdaxdr.pyx	89.81% <62.50%> (-0.16%)	⬇️
package/MDAnalysis/lib/formats/libdcd.pyx	88.48% <66.66%> (-0.85%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22612de...5e9d11b. Read the comment docs.

[orbeckst]

Good that we had a look at this again. I think this looks right but please see comments. If it's the way that I think it is then we need to add a bit of documentation to the code and at least the DCDReader.

However, I am not really keen on making DCDFile.seek() behave differently from any other seek. Could we do the following: Make the second part of DCDFile.seek() starting at self.reached_eof = False a private method _seek() that only checks that its input frame is 0 <= frame <= self.n_frames. DCDFile.__setstate__ can call _seek() but DCDFile.seek() would check 0 <= frame < self.n_frames before calling _seek() and so continue to behave like all the other Reader.seek()` methods.

[orbeckst]

Is this supposed to allow to seek to the end of the file? If so, add a comment, because under normal circumstances I expect a 0-based index for a sequence to fail when it hits len(sequence).

[orbeckst]

If it is possible to seek to the end of the file (after the last coordinate frame) with

dcd.seek(len(dcd))

then state it in the doc string. This is a fairly odd thing to allow so it should be documented.

[orbeckst]

Did you mean "previous frame" instead of "frame ahead"?

[orbeckst]

Switch the decision in the if statement to make the more common situation come first

if current_frame < self.offsets.size:
     ...
elif current_frame == self.offsets.size:
    ...
else:
    #pragma: no cov
    raise RuntimeError("Invalid frame number {} > {} -- this should not happen.".format(current_frame, self.offsets.size)

and add defensive code.

[orbeckst]

Add comment under which circumstances we have to do funny things – namely when we need to serialize at the end of the trajectory.

[yuxuanzhuang]

I makes DCDFile to behave the same as XDRFile, i.e. not messing up with seek, but checking during __setstate__

[yuxuanzhuang]

The pragma: no cover is not honered in cython files (cython/cython#3680)
Any reason we have to check for that here (aside from for the completeness), since it will be eventually checked in seek.

[orbeckst]

This looks like a good solution, just doing the special casing inside __setstate__.

It's ok that "no cover" didn't work, thanks for finding out why.