{"action":"created","issue":{"url":"https://api.github.com/repos/biojava/biojava/issues/359","labels_url":"https://api.github.com/repos/biojava/biojava/issues/359/labels{/name}","comments_url":"https://api.github.com/repos/biojava/biojava/issues/359/comments","events_url":"https://api.github.com/repos/biojava/biojava/issues/359/events","html_url":"https://github.com/biojava/biojava/issues/359","id":124868967,"number":359,"title":"mmCIF SimpleMMcifConsumer incorrectly replaces residue numberings.","user":{"login":"larsonmattr","id":7458854,"avatar_url":"https://avatars.githubusercontent.com/u/7458854?v=3","gravatar_id":"","url":"https://api.github.com/users/larsonmattr","html_url":"https://github.com/larsonmattr","followers_url":"https://api.github.com/users/larsonmattr/followers","following_url":"https://api.github.com/users/larsonmattr/following{/other_user}","gists_url":"https://api.github.com/users/larsonmattr/gists{/gist_id}","starred_url":"https://api.github.com/users/larsonmattr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/larsonmattr/subscriptions","organizations_url":"https://api.github.com/users/larsonmattr/orgs","repos_url":"https://api.github.com/users/larsonmattr/repos","events_url":"https://api.github.com/users/larsonmattr/events{/privacy}","received_events_url":"https://api.github.com/users/larsonmattr/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2016-01-05T00:25:12Z","updated_at":"2016-01-06T19:22:55Z","closed_at":null,"body":"The SimpleMMcifConsumer class currently replaces residue numbers for Group instances when it parses the _pdbx_poly_seq_scheme.  \r\n\r\n1. In multi-model structures, this replacement only searches through the previous 1..N-1 models, as the last current_model does not get added until documentEnd() is reached.  This will cause the models to have out-of-sync numberings.\r\n\r\n2. With the example of the model PDB:2KSA, performing this renumbering based on the pdbx_poly_seq_scheme actually incorrectly renumbers residues.  Instead, it seems sufficient to rely on the original _atom_record numbers that would then agree with the PDB entry for 2KSA.  All the current unit tests for pdb/mmcif consistency pass with the renumbering by _pdbx_poly_seq_scheme disabled.\r\n\r\nAt least fix for (1) would correct the issue, but I am wondering if it would be best to avoid this issue completely by preventing renumbering as suggested by (2)?\r\n\r\nI can submit a pull request with related changes but would like a comment if disabling the renumbering should be part of this fix.  "},"comment":{"url":"https://api.github.com/repos/biojava/biojava/issues/comments/169427408","html_url":"https://github.com/biojava/biojava/issues/359#issuecomment-169427408","issue_url":"https://api.github.com/repos/biojava/biojava/issues/359","id":169427408,"user":{"login":"josemduarte","id":6000474,"avatar_url":"https://avatars.githubusercontent.com/u/6000474?v=3","gravatar_id":"","url":"https://api.github.com/users/josemduarte","html_url":"https://github.com/josemduarte","followers_url":"https://api.github.com/users/josemduarte/followers","following_url":"https://api.github.com/users/josemduarte/following{/other_user}","gists_url":"https://api.github.com/users/josemduarte/gists{/gist_id}","starred_url":"https://api.github.com/users/josemduarte/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/josemduarte/subscriptions","organizations_url":"https://api.github.com/users/josemduarte/orgs","repos_url":"https://api.github.com/users/josemduarte/repos","events_url":"https://api.github.com/users/josemduarte/events{/privacy}","received_events_url":"https://api.github.com/users/josemduarte/received_events","type":"User","site_admin":false},"created_at":"2016-01-06T19:22:55Z","updated_at":"2016-01-06T19:22:55Z","body":"Indeed 1) is clearly a bug. Regarding 2) it does sound like a reasonable approach in principle. However I don't fully understand what part of the code you refer to. I suppose it is the `replaceGroupSeqPos()` method? Could you point to the which part of the code you are referring to, or alternatively create a branch with a test demonstrating the issue for 2ksa?\r\n"}}