Introduce universally Timestep.time as part of the Reader/Timestep API

[dotsdl]

Subtask of #252:

Introduce universally Timestep.time (as part of the API):

• If the trajectory records time, it will contain the current frame's time (in MDA units)
• If the trajectory records time between frames dt and frame numbers N, it will calculate the time as N * dt.
• If the trajectory does not contain time information then log a warning and pretend that dt = 1 and use the N * dt calculated time.

deprecate Reader.time (for 1.0)

[richardjgowers]

So with this I've realised that Timesteps don't actually have a pointer to their Reader. So to give time as dt * frame is impossible because the Timestep can't see Reader.dt.

[richardjgowers]

Ah, I've just seen #307 fixes this, so this is reliant on that.

[dotsdl]

So with this I've realised that Timesteps don't actually have a pointer to their Reader. So to give time as dt * frame is impossible because the Timestep can't see Reader.dt.

Should we give Timesteps a pointer to their Reader? What are the reasons to do it / not to do it?

[richardjgowers]

I actually started hacking on this today, what I did was make Reader.dt a property that updated its timestep.

I do like that Timesteps can work alone and are essentially a simple container, but maybe adding an optional hook to their Reader might simplify things.

[orbeckst]

I'd leave Timesteps decoupled from Readers if possible. I don't think that anything will break horribly if it is included (unless we're talking pickling of Timestep instances...) but it is cleaner and it forces us to think properly through how to design both Timestep and Readers. #307 is a case in point and see #350 for discussion.

[richardjgowers]

With this, I've changed it so that single frame readers now return a dt of 1.0 ps. They previously raised a KeyError. I figure this would make it more uniform for readers to always have a dt which you can kwarg in if you want. Might also help with the ChainReader issues. #309

Is this logic terrible?

[orbeckst]

Makes sense and I don't think it will be a problem. Throwing the KeyError is pretty unhelpful (at least it should have been NoDataError or similar) but in the grand scheme of things not having to catch an exception for something one is unlikely to care about seems ok to me. Do add a warning and a version changed to the single frame reader Timestep, though.