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

Add support for oxDNA file parsing #536

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jan-stevens
Copy link
Member

@jan-stevens jan-stevens commented Aug 17, 2023

This PR implements a file parser of oxDNA configuration files to read in rotational frames for residues. The initial goal is to use the file parser to implement a frame-based backmapping processer in Polyply.

Info on the file format can be found here.

Copy link
Member

@pckroon pckroon left a comment

Choose a reason for hiding this comment

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

In general I like it. I'm not quite sure how vermouth/martinize would use this, but that's fine.
I'm still missing tests; and some error handling. If I have a misformatted oxdna file, can you tell me which line (and file) caused what exception?

from ..molecule import Molecule


def read_oxDNA(file_name, exclude=()):
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest the actual parser doesn't accept a file name, but rather an iterator of lines. This makes reading e.g. network streams or compressed files easier/possible in the future. A wrapper method can open the file.

Comment on lines +37 to +42
Returns
-------
vermouth.molecule.Molecule
The parsed molecules. Will not contain edges.
"""
molecule = Molecule()
Copy link
Member

Choose a reason for hiding this comment

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

The PDB parser returns a System, and I think that would also make more sense here?

Comment on lines +53 to +54
has_b= first_line.count(' ') > 2
has_n = first_line.count(' ') > 5
Copy link
Member

Choose a reason for hiding this comment

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

Is split safer/better/easier?


# Start parsing the file in earnest. And let's not forget the first
# line.
for line in chain([first_line], oxDNA):
Copy link
Member

Choose a reason for hiding this comment

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

Nice use of chain 👍


# Start parsing the file in earnest. And let's not forget the first
# line.
for line in chain([first_line], oxDNA):
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick suggestion

Suggested change
for line in chain([first_line], oxDNA):
for idx, line in enumerate(chain([first_line], oxDNA)):

@@ -0,0 +1,19 @@
# -*- coding: utf-8 -*-
# Copyright 2018 University of Groningen
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2018 University of Groningen
# Copyright 2023 University of Groningen

@@ -0,0 +1,75 @@
# -*- coding: utf-8 -*-
# Copyright 2018 University of Groningen
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2018 University of Groningen
# Copyright 2023 University of Groningen

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