-
Notifications
You must be signed in to change notification settings - Fork 652
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
Port Coordinates test to new BaseReader/Writer Test classes #516
Comments
@kain88-de could you explain what needs to be done here? |
@kain88-de From what I understand, there are What I don't understand is how to, if I need to, first generate the files that I have to test against ( |
@utkbansal The files are there already. They have been built with the software that usually generate them (simulation engines mainly). If I well remember, the test classes have a bunch of arguments to adapt to the content of the existing reference files. |
Okay, so which file should I start porting first? |
Among the file formats, find one that you understand (the file format, not the code at first). Then see if you understand the code that you are going to test: the readers are critical parts of the code, but there is rarely to much dark magic in them. I would suggest the GRO reader. |
Okay, so I know nothing about any of the file formats. What should I be reading, if say I start out with the GRO reader? |
The GRO format is described on the gromacs website. One caveat on what is on the page: the precision of the coordinate can vary; it is defined by the first atom line. If I well remember, the code is clear enough on that aspect. |
Okay, thanks. I'll send a PR when I'm ready with something. |
For each format that is added we need to generate a small test file that follows strict conventions that are tested. The conventions are listed in the reference. You can have a look at the XYZ tests. |
https://github.com/MDAnalysis/mdanalysis/tree/develop/testsuite/MDAnalysisTests/data/coordinates You'll need to create a new |
@richardjgowers From what I understand by reading the code, the current |
yes |
I would like to work on some parts of this issue. |
@Akshay0724 The classes that read and write coordinates are tested in a non consistent way. We would like them to be tested consistently with tests based on the |
Is there another format that I can work on? |
@utkbansal from the list above, you probably want to pick one with a Writer so you can easily generate the new reference file, so maybe PQR. |
As discussed in today's call with @yuxuanzhuang, the list of readers/writers that don't use BaseReaderTest/Writer classes has unfortunately grown over time. We probably should do a push to get all this done, any thoughts on putting this down as a 2.0 goal? |
For GSoC starters that might be interested in this issue. We would be looking for folks to tackle one of these tests at the time. |
I've worked with lammps files before, maybe I can take that up.! |
@IAlibay, is it okay if I start scripting something for LAMMPS data files? Perhaps there is another format that is more important to be merged with the base test. (?) |
I realise this is an important thing to reach, however being not user facing how do we feel about moving the goal from 2.0 to 2.1? We can do our best to move towards addressing it (especially if GSoC students want to work on them), but we don't make it a compulsory target for 2.0? (I'm trying to see what we can do to streamline the 2.0 release). |
I realise I forgot to tag @MDAnalysis/coredevs here. Unless there's any objections, I'm moving this to the 3.0 milestone (if we can get it done early then great, but otherwise it's best it doesn't hinder the 2.0 release). |
That is not affecting users, it does not need to be on a major release. 2.x would be fine. But OK for 3.0. |
pytest --disable-pytest-warnings Errors:
_____________________________________ ERROR collecting MDAnalysisTests/analysis/test_align.py _____________________________________
ImportError while importing test module '/home/heet/Work/Development/mdanalysis/testsuite/MDAnalysisTests/analysis/test_align.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../importlib/__init__.py:126: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
/home/heet/Work/Development/mdanalysis/testsuite/MDAnalysisTests/analysis/test_align.py:27: in <module>
from MDAnalysis.analysis.align import HAS_BIOPYTHON
E ImportError: cannot import name 'HAS_BIOPYTHON' from 'MDAnalysis.analysis.align' (/home/heet/anaconda3/envs/mdanalysis-dev/lib/python3.10/site-packages/MDAnalysis/analysis/align.py)
____________________________________ ERROR collecting MDAnalysisTests/analysis/test_leaflet.py ____________________________________
ImportError while importing test module '/home/heet/Work/Development/mdanalysis/testsuite/MDAnalysisTests/analysis/test_leaflet.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../importlib/__init__.py:126: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
/home/heet/Work/Development/mdanalysis/testsuite/MDAnalysisTests/analysis/test_leaflet.py:29: in <module>
from MDAnalysis.analysis.leaflet import LeafletFinder, optimize_cutoff, HAS_NX
E ImportError: cannot import name 'HAS_NX' from 'MDAnalysis.analysis.leaflet' (/home/heet/anaconda3/envs/mdanalysis-dev/lib/python3.10/site-packages/MDAnalysis/analysis/leaflet.py)
_________________________________ ERROR collecting MDAnalysisTests/analysis/test_nucleicacids.py __________________________________
ImportError while importing test module '/home/heet/Work/Development/mdanalysis/testsuite/MDAnalysisTests/analysis/test_nucleicacids.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../importlib/__init__.py:126: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
/home/heet/Work/Development/mdanalysis/testsuite/MDAnalysisTests/analysis/test_nucleicacids.py:30: in <module>
from MDAnalysis.analysis.nucleicacids import (NucPairDist, WatsonCrickDist,
E ImportError: cannot import name 'MajorPairDist' from 'MDAnalysis.analysis.nucleicacids' (/home/heet/anaconda3/envs/mdanalysis-dev/lib/python3.10/site-packages/MDAnalysis/analysis/nucleicacids.py)
___________________________________ ERROR collecting MDAnalysisTests/core/test_residuegroup.py ____________________________________
ImportError while importing test module '/home/heet/Work/Development/mdanalysis/testsuite/MDAnalysisTests/core/test_residuegroup.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../importlib/__init__.py:126: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
/home/heet/Work/Development/mdanalysis/testsuite/MDAnalysisTests/core/test_residuegroup.py:29: in <module>
from MDAnalysis.core.topologyattrs import HAS_BIOPYTHON
E ImportError: cannot import name 'HAS_BIOPYTHON' from 'MDAnalysis.core.topologyattrs' (/home/heet/anaconda3/envs/mdanalysis-dev/lib/python3.10/site-packages/MDAnalysis/core/topologyattrs.py) Overall summery:============================================================ short test summary info =============================================================
ERROR analysis/test_align.py
ERROR analysis/test_leaflet.py
ERROR analysis/test_nucleicacids.py
ERROR core/test_residuegroup.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 4 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
==================================================== 3 skipped, 6 warnings, 4 errors in 9.43s ====================================================
|
@HeetVekariya looks like you aren't using a development installation of MDAnalysis, none of these are actual test failures. I would suggest opening a topic on the discussion tracker about this. |
Since #474 we have a Test Class that all coordinates formats should share for testing.
This issue is here to track progress about it
The text was updated successfully, but these errors were encountered: