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

Load DCD and LAMMPS on python 3 #1454

Closed
jbarnoud opened this issue Jul 8, 2017 · 2 comments
Closed

Load DCD and LAMMPS on python 3 #1454

jbarnoud opened this issue Jul 8, 2017 · 2 comments
Assignees
Labels

Comments

@jbarnoud
Copy link
Contributor

jbarnoud commented Jul 8, 2017

Now that #1372 is merged, there is no reason not to load DCD on python 3. Currently, python 3 tries to load the DCD and LAMMPS reader and let it go if it fails. See https://github.com/MDAnalysis/mdanalysis/blob/develop/package/MDAnalysis/coordinates/__init__.py. The DCD and LAMMPS reader should be loaded as the others, now.

With the two modules loaded normally, there is no reason to skip the tests involving DCD. The skips should be phased out.

@utkbansal
Copy link
Member

@jbarnoud So basically all skips that involve checking DCD availability have to be removed? It should work with both Python versions.

@jbarnoud
Copy link
Contributor Author

jbarnoud commented Jul 9, 2017

The tests should run in both python version. They do not have to pass in both versions, though.

Note that, because of how the DCD reader was imported, the tests probably do run on python 3 now. Indeed, the library tries to import the DCD reader, succeed, and therefore do not skip the tests.

However, skipping the tests should not be an option. In practice, it means removing the try/except in mda.coordinates.__init__, and removing all the skips that are there because of DCD.

All the skips are identical, so it should even be doable automatically. It may be better for you to do it progressively in your other PRs, though. To avoid having to deal with conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants