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

move header writing in gmx files to a system level meta attribute #658

Merged
merged 15 commits into from
Feb 12, 2025

Conversation

csbrasnett
Copy link
Collaborator

instead of collecting the header for gmx files as a list, move it to a system.meta["header"] list that can be more dynamically added to

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.

Nice work.
I do think there's some snakes in the grass where the headers of separate molecules start to affect each other though. https://github.com/marrink-lab/vermouth-martinize/pull/658/files#diff-4071134fd6f80afcd683f455e2558cc2fbd683b801fe6bc78ebc28fb86cdf655R225 modifies the header for the entire system, rather than just the one molecule.
Not sure that's a real issue.

vermouth/gmx/topology.py Outdated Show resolved Hide resolved
vermouth/gmx/topology.py Show resolved Hide resolved
vermouth/gmx/topology.py Outdated Show resolved Hide resolved
- needed to move the command comment to just after the system is made, otherwise the output files have things written the wrong way around
@csbrasnett
Copy link
Collaborator Author

csbrasnett commented Feb 7, 2025

Moved things around. Moving the secondary structure part to dssp made a slight mess because it then wrote that ahead of the command used, which IMO is important to keep as the first line in the itp file

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.

Small comments

bin/martinize2 Outdated Show resolved Hide resolved
vermouth/dssp/dssp.py Outdated Show resolved Hide resolved
vermouth/tests/test_dssp.py Outdated Show resolved Hide resolved
vermouth/dssp/dssp.py Outdated Show resolved Hide resolved
bin/martinize2 Outdated Show resolved Hide resolved
pckroon
pckroon previously approved these changes Feb 10, 2025
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.

LGTM!

@csbrasnett
Copy link
Collaborator Author

Thanks, working on fixing the test and there's something very weird happening with the line length stuff... will be done eod...

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.

Awesome

@pckroon pckroon merged commit ac93f62 into master Feb 12, 2025
10 checks passed
@pckroon pckroon deleted the system-meta branch February 12, 2025 09:59
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