timeseries() for all Readers

[orbeckst]

Changes made in this Pull Request:

• add timeseries() coordinate extractor for all readers
• cherry-picked @richardjgowers old patch from PR Adds Reader.timeseries #1400

Note that there's still some work to be done

• make MemoryReader.timeseries, DCD.timeseries and this ProtoReader.timeseries fully consistent
• allow all combinations for order (not only "FAC")

PR Checklist

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

[codecov]

Codecov Report

Merging #2271 into develop will decrease coverage by 0.12%.
The diff coverage is 6.66%.

@@             Coverage Diff             @@
##           develop    #2271      +/-   ##
===========================================
- Coverage    89.66%   89.54%   -0.13%     
===========================================
  Files          172      172              
  Lines        21398    21429      +31     
  Branches      2785     2788       +3     
===========================================
+ Hits         19186    19188       +2     
- Misses        1616     1645      +29     
  Partials       596      596

Impacted Files	Coverage Δ
package/MDAnalysis/coordinates/memory.py	96.02% <ø> (ø)	⬆️
package/MDAnalysis/coordinates/base.py	91.56% <6.66%> (-1.74%)	⬇️
coordinates/base.py	92.94% <0%> (-1.86%)	⬇️

---

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 f83498e...1913f73. Read the comment docs.

[richardjgowers]

I think this might be double copying in the case of a non-memoryview. I think maybe either np.array or np.asarray lets you specify copy=True which will end up with one copy max.

[orbeckst]

I think you're right.

But note that you gave the MemoryReader its own timeseries method

mdanalysis/package/MDAnalysis/coordinates/memory.py

Line 475 in f83498e

def timeseries(self, asel=None, start=0, stop=-1, step=1, order='afc', format=None):

Would be nice if we could do everything consistently in base.

[orbeckst]

Btw, I think I misunderstood your comment.... you were not referring to MemoryReader at all, were you?

[orbeckst]

np.asanyarray() does not have copy=True. np.array(a, copy=True) is equivalent to np.copy(a).

Do you want to do something like

x = ts.positions[atom_numbers]
if is_copy(x):
   coordinates[i] = x
else:
   coordinates[i] = x.copy()

(and where I don't know how to do is_copy(x))?

[richardjgowers]

This is cool, but maybe it should be called coordinates_timeseries? unless it's obvious that the only timeseries of interest is the coordinates? I also think maybe the function should be made more general to be a timeseries of all time data, so box info, velocities etc? This would make the Reader -> MemoryReader transformation trivial. So returning something like a numpy structured array? Not sure what a good container for this is though...

[orbeckst]

I know, it's not general enough at the moment. However, it is consistent with the MemoryReader

mdanalysis/package/MDAnalysis/coordinates/memory.py

Lines 475 to 478 in f83498e

	def timeseries(self, asel=None, start=0, stop=-1, step=1, order='afc', format=None):
	"""Return a subset of coordinate data for an AtomGroup in desired
	column order/format. If no selection is given, it will return a view of
	the underlying array, while a copy is returned otherwise.

[orbeckst]

@richardjgowers thanks for the comments. I don't have a ton of time on my hands to work on the PR. I mainly put it up so that some of your old work did not get junked. (I came across the old PR for some other reasons and just noticed your old timeseries code.)

It would be nice to have a consistent way to pull trajectory data into arrays. I agree with your comment that it should be possible to not only get coordinates but also velocities, forces, box information – if available. Auxiliaries would be neat, too, but they aren't used that much as far as I can tell, so one could leave them out, until we come up with a killer-app for them (... I am thinking alchemical free energy calculations analysis would benefit tremendously, but that's another thread).

[orbeckst]

Closing as stale (and I don't even know if the PR/issue is still relevant)