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

Deprecate Timestep as argument for writers #2043

Closed
5 tasks
orbeckst opened this issue Aug 12, 2018 · 4 comments
Closed
5 tasks

Deprecate Timestep as argument for writers #2043

orbeckst opened this issue Aug 12, 2018 · 4 comments
Labels
Component-Writers deprecation Deprecated functionality to give advance warning for API changes. usability
Milestone

Comments

@orbeckst
Copy link
Member

orbeckst commented Aug 12, 2018

As discussed in #206 we want to streamline the trajectory writer interface so that

Writer.write(arg)

only accepts AtomGroup or Universe as arg but not Timestep anymore. (We can't think of a good reason when we would have a Timestep but not an AG.)

  • deprecate the part of base.Writer.write()
    if isinstance(obj, Timestep):
    that can directly take a Timestep
  • check all specific Writers that we don't override
  • add comments that in 1.0 supplying a Timestep will raise a TypeError
  • add tests for all writers that check for the deprecation warning (and can later be turned into tests that check for TypeError)
  • check docs and replace deprecated usage
@orbeckst
Copy link
Member Author

I am adding this to 0.19.0 – but this can be targeted to a 0.19.x later.

@orbeckst orbeckst added the deprecation Deprecated functionality to give advance warning for API changes. label Aug 12, 2018
@richardjgowers richardjgowers modified the milestones: 0.19.0, 0.19.x Oct 4, 2018
richardjgowers added a commit that referenced this issue Oct 16, 2018
identified which Writers can currently use a ts (lots don't anyway)
orbeckst pushed a commit that referenced this issue Apr 3, 2019
- identified which Writers can currently use a ts (lots don't anyway)
- deprecated Timestep as argument to Writer.write
- Update package/MDAnalysis/coordinates/base.py
- Update package/MDAnalysis/coordinates/XYZ.py
- Update package/MDAnalysis/coordinates/GRO.py
- add test_writer_api.py
- removed obsolete test
@orbeckst
Copy link
Member Author

orbeckst commented Mar 6, 2020

We might have missed the boat on deprecating to get rid of 1.0.

So I think we should deprecate in 1.0 and remove in 2.0.

@orbeckst orbeckst modified the milestones: 0.20.x, 1.0 Mar 6, 2020
richardjgowers added a commit that referenced this issue Jun 5, 2020
- identified which Writers can currently use a ts (lots don't anyway)
- deprecated Timestep as argument to Writer.write
- Update package/MDAnalysis/coordinates/base.py
- Update package/MDAnalysis/coordinates/XYZ.py
- Update package/MDAnalysis/coordinates/GRO.py
- add test_writer_api.py
- removed obsolete test
richardjgowers added a commit that referenced this issue Jun 6, 2020
- identified which Writers can currently use a ts (lots don't anyway)
- deprecated Timestep as argument to Writer.write
- Update package/MDAnalysis/coordinates/base.py
- Update package/MDAnalysis/coordinates/XYZ.py
- Update package/MDAnalysis/coordinates/GRO.py
- add test_writer_api.py
- removed obsolete test
richardjgowers added a commit that referenced this issue Jun 8, 2020
* deprecate Timestep argument to Writers (issue #2043)

Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
Co-authored-by: Lily Wang <31115101+lilyminium@users.noreply.github.com>
@orbeckst
Copy link
Member Author

@richardjgowers @IAlibay what is left over after merge of PR #2110?

Close or re-target to later milestone.

@IAlibay
Copy link
Member

IAlibay commented Jun 10, 2020

I think #2110 should have fixed all this now. I'll close.

@IAlibay IAlibay closed this as completed Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component-Writers deprecation Deprecated functionality to give advance warning for API changes. usability
Projects
None yet
Development

No branches or pull requests

3 participants