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

Lammps dump reader #1889

Merged
merged 8 commits into from
May 15, 2018
Merged

Conversation

richardjgowers
Copy link
Member

Fixes #1888

Changes made in this Pull Request:

  • adds support for lammps dump reader

Still needs docs, tests for a triclinic system

@orbeckst
Copy link
Member

orbeckst commented May 7, 2018

Btw, we can't merge this into a 0.18.1 because it is a new feature.

Do you want to hammer out a quick 0.18.1 and then move forward to 0.19.0 or 1.0.0?

@richardjgowers
Copy link
Member Author

@orbeckst yeah one day I'll manage to get a patch release out. There's no point holding this back just so we can have an X.1 release, if this gets finished and merged it'll force 19.0 I guess. We should have a release soon

Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

semantic versioning and testing compressed files

Will automatically convert positions from their scaled/fractional
representation to their real values.

.. versionadded:: 0.18.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has to be 0.19.0 because it is a new feature.

self._file.close()

def _read_frame(self, frame):
self._file.seek(self._offsets[frame])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does seek work on compressed files?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently yes, we test seeking and reading which relies on this


Only reads atom ids. Sets all masses to 1.0.

.. versionadded:: 0.18.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to 0.19.0

assert ts.frame == i
assert ts.data['step'] == i * 500

def test_seeking(self, u, reference_positions):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also test compressed files: zip, gzip, bz2



class TestLammpsDumpReader(object):
@pytest.fixture()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could save your data file as bz2 compressed and then create fixtures that create the uncompressed, gz, and zip versions for the test.

@orbeckst
Copy link
Member

Can you please fix the docs https://travis-ci.com/MDAnalysis/mdanalysis/jobs/123399600 and the linter is also unhappy.

@orbeckst
Copy link
Member

@richardjgowers before you merge this, consider if you don't want to push out a quick 0.18.1 – we had two users already complaining about #1872 . If we move to 0.19.0 then invariably it will take much longer to get a new release out.

@richardjgowers
Copy link
Member Author

richardjgowers commented May 11, 2018 via email

@orbeckst
Copy link
Member

orbeckst commented May 11, 2018 via email

@richardjgowers
Copy link
Member Author

richardjgowers commented May 11, 2018 via email

@richardjgowers richardjgowers merged commit 2e9db81 into develop May 15, 2018
@richardjgowers richardjgowers deleted the issue-18something-lammps_dump_reader branch May 15, 2018 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants