Skip to content

JMK parser fix to account for xmipp pseudoatom writing changes #134

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

Draft
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

jamesmkrieger
Copy link
Collaborator

Accounts for I2PC/xmipp#664

With both changes, the TestHEMNMA_1 works fine

Copy link
Collaborator

@MohamadHarastani MohamadHarastani left a comment

Choose a reason for hiding this comment

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

I would ask Remi to verify since he wrote this file. I will approve it from my side.

@jamesmkrieger
Copy link
Collaborator Author

Thanks, Mohamad.

Remi, can you also look at the changes on the xmipp side, please?

@jamesmkrieger
Copy link
Collaborator Author

We're still thinking further about this on the xmipp side, so I'm converting this to draft

@jamesmkrieger jamesmkrieger marked this pull request as draft November 15, 2022 09:07
@mms29
Copy link
Collaborator

mms29 commented Nov 20, 2022

Hi James,
Sorry for the late reply I was on holidays.
The PDB parser is based on the specification found here : https://www.cgl.ucsf.edu/chimera/docs/UsersGuide/tutorials/pdbintro.html
Your modifications do not follow this specification so I assume it will not work for some PDBs (Although it might work in some cases)
It would help for me to have some background on the issue, what was the error of TestHEMNMA_1 you mentioned ?

@MohamadHarastani
I haven't used this pdb parser (I use continuousflex/protocols/utilities/pdb_handler)
I don't know who wrote this but it is similar to mine for reading and writing

@jamesmkrieger
Copy link
Collaborator Author

There is no error with the tests that I’m fixing. Rather, I’m mentioning the test to say that I’m not breaking anything.

The idea here is indeed to aim towards having a pseudoatom pdb file that actually follows the pdb format. This parser that I edited is the specific one for that, which in theory could probably be retired. David Strelak and I were thinking we could do the same on the Xmipp side.

@mms29
Copy link
Collaborator

mms29 commented Nov 22, 2022

I believe PDB file with atoms or pseudoatoms should follow the exact same specification, maybe only the residue name will be specific to pseudoatoms, but the rest should be the same. I think this is how it is done so far in continuousflex. I don't understand the problem you are having and why you should modify this PDB reader to pseudoatoms pdb files.
Also, I believe this function is referred from the NMA protocol and is not dedicated to pseudoatoms files, so by doing this modification it might break the NMA protocol for some atomic PDBs?

@jamesmkrieger
Copy link
Collaborator Author

They should be but they aren’t actually. I’ll have a look at this all more carefully

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.

3 participants