[MINOR] Enable pdb_splitmodel to handle more than 9999 models
#171
Conversation
update model_no indexing method
There was a problem hiding this comment.
I personally disagree with this for the same reason expressed by @JoaoRodrigues here:
However, I understand what @amjjbonvin argued after. Still, I disagree with this PR as it deviates from the official PDB format. Users can quickly create their branch to apply this change locally if needed.
In any case, if this PR is to be accepted it should be as @JoaoRodrigues said afterwards in the issue and not in the current form of line[6:].
Good point, I'd still keep then the model number starting at position 11 but open-ended. Otherwise you will indeed parse bad files with numbers where they should not be and they might be interpreted wrong by different tools, e.g.
Cheers!
|
However, I understand what @amjjbonvin <https://github.com/amjjbonvin> argued after. Still, I disagree with this PR as it deviates from the official PDB format. Users can quickly create their branch to apply this change locally if needed.
I do not agree - this is a simple solution, provide we indeed keep the correct column alignment. But this model number is not very critical.
And in addition the PDB only accepts mmCIF format these days.
|
Modified to read the model number from column 11 on
There was a problem hiding this comment.
This PR is a small edit but if merged represents a big shift in the philosophy of this project. pdb-tools has also strictly adhered to the PDB File Format and there is a lot of value on that.
My first suggestion would be to not merge this and to avoid using pdb-tools for this use case (with more than 9999 models, and any other that falls outside the format).
However, considering this project lives in the haddocking organization and that this was built by the group, to help the group, I think the need of the group should go above the PDB File Format.
Even tho it's not a major change, imo pdb-tools should be bumped to v3.0.0 if this gets merged, considering my point above.
|
And to add to that, the current version, when encountering more than 9999 models will overwrite over models, e.g. model 11000 will be written as model 1000, overwriting the first instance of model 1000. And this without warning. |
|
And while at it, So to be consistent we could use here instead: |
Yes. This is exactly what I found trying to do clustering with hd3 with more than 9999 models. |
pdb_splitmodel to handle more than 9999 models
To solve this #170 issue