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

update the Trajectory API docs: Writers #3392

Closed
orbeckst opened this issue Aug 19, 2021 · 0 comments · Fixed by #3393
Closed

update the Trajectory API docs: Writers #3392

orbeckst opened this issue Aug 19, 2021 · 0 comments · Fixed by #3393

Comments

@orbeckst
Copy link
Member

orbeckst commented Aug 19, 2021

Expected behavior

API docs https://docs.mdanalysis.org/2.0.0-dev0/documentation_pages/coordinates/init.html#trajectory-api and code (namely coordinates.base.WriterBase etc) agree

Actual behavior

API docs require start, stop, step, dt but the writers, starting with

class WriterBase(IOBase, metaclass=_Writermeta):
and (#3294 (comment)) and other main writers (DCD, XDR, LAMMPS), don't use start/step/delta (or even has it as a parameter for init).

The closest we have is step and dt for the DCDWriter, although worth nothing that it's not immediately clear that step actually does anything.

Suggested resolution

It's possible I'm missing something, but since it doesn't look like writers don't meet the API's requirements to have start, stop, step, dt (either as parameters to init or as class attributes) we just clean up the API docs to match our code.

Background

See eg #3294 (comment)

Current version of MDAnalysis

  • Which version are you using? 2.0.0-dev
@orbeckst orbeckst added this to the 2.0 milestone Aug 19, 2021
@orbeckst orbeckst self-assigned this Aug 19, 2021
orbeckst added a commit that referenced this issue Aug 19, 2021
- fix #3392
- writers are not required to process start, stop, step, which is what
  all our code does anyway
- clarified that writer needs to ignore kwargs that they cannot process
- clarified that __init__ can delay actual I/O until Write
@orbeckst orbeckst mentioned this issue Aug 19, 2021
3 tasks
IAlibay pushed a commit that referenced this issue Aug 20, 2021
Fixes #3392

* change Coordinate Writer API
- writers are not required to process start, stop, step, which is what
  all our code does anyway
- clarified that writer needs to ignore kwargs that they cannot process
- clarified that __init__ can delay actual I/O until Write

* doc cleanup in coordinates and converters
- fix links to Trajectory API
- link coordinates.ConverterBase to MDAnalysis.converters
- minor reST fixes
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.

1 participant