rinikerlab / PyGromosTools

This package is a python library with tools for the Molecular Simulation - Software Gromos. It allows you to easily set up, manage and analyze simulations in python.
https://rinikerlab.github.io/PyGromosTools/
MIT License
16 stars 14 forks source link

Fix loading trc.gz files, and writing trc without box info #307

Closed fwaibl closed 1 year ago

fwaibl commented 1 year ago

Hi, I am using PyGromosTools to read and write trajectories in .trc.gz format. While doing so, I encountered two different issues, one during writing and one during reading.

Loading

When loading a .trc.gz file, the file was previously unzipped in the same folder. This means that write access was needed to read a file. For instance, I got a permission error when trying to load a trajectory from a folder where I did not have write access:

import pygromos.files.trajectory.trc
traj = pygromos.files.trajectory.trc.Trc(traj_path="/fileserver/other_user/example.trc.gz")

This commit changes the behavior to use the gzip module instead.

Writing

When writing a .trc file without box info, there was previously a TypeError in generate_entry_for_frame.

>>> import pygromos.files.trajectory.trc
>>> traj = pygromos.files.trajectory.trc.Trc(traj_path="test.trc.gz")
>>> traj.unitcell_lengths = None
>>> traj.save("test2.trc.gz")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/localhome/fwaibl/.conda/envs/pygromos/lib/python3.10/site-packages/pygromos/files/trajectory/trc.py", line 544, in save
    out_path = self.write_trc(out_path)
  File "/localhome/fwaibl/.conda/envs/pygromos/lib/python3.10/site-packages/pygromos/files/trajectory/trc.py", line 400, in write_trc
    array_list.append(self.generate_entry_for_frame(i))
  File "/localhome/fwaibl/.conda/envs/pygromos/lib/python3.10/site-packages/pygromos/files/trajectory/trc.py", line 416, in generate_entry_for_frame
    if not (self.unitcell_lengths is None and len(self.unitcell_lengths) > 0):
TypeError: object of type 'NoneType' has no len()

This fixes the error and adds a unit test to write files without box information.

Please let me know what you think about it.

Best regards, Franz Waibl

codecov[bot] commented 1 year ago

Codecov Report

Merging #307 (f0f8c14) into main (f5e59f9) will decrease coverage by 0.17%. The diff coverage is 95.42%.

:exclamation: Current head f0f8c14 differs from pull request most recent head fe62d2c. Consider uploading reports for the commit fe62d2c to get more accurate results

@@            Coverage Diff             @@
##             main     #307      +/-   ##
==========================================
- Coverage   54.56%   54.40%   -0.17%     
==========================================
  Files          93       93              
  Lines       14384    14349      -35     
==========================================
- Hits         7849     7806      -43     
- Misses       6535     6543       +8     
Flag Coverage Δ
unittests 54.40% <95.42%> (-0.17%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pygromos/files/coord/cnf.py 45.36% <92.30%> (+0.47%) :arrow_up:
pygromos/files/trajectory/trc.py 88.25% <95.68%> (-1.71%) :arrow_down:
pygromos/files/blocks/_general_blocks.py 81.81% <100.00%> (ø)

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

fwaibl commented 1 year ago

@MTLehner could you have a look at this?

fwaibl commented 1 year ago

@katzberger @MTLehner I also wrote some updates for the PDB writing (regarding fixed-width columns with long residue names), and a pure-Python version of the TRC loading and writing functions, that is IMO much simpler than the current one, and also faster for my test file. If you want, I could add those to the pull request, or make a new one with all the changes in it. However, there is probably more to discuss than for this PR, since especially the PDB formatting is rather opinionated.

MTLehner commented 1 year ago

Very nice cleanup and fixing