Implemented extraction of chains from DMS formats by ianmkenney · Pull Request #2872 · MDAnalysis/mdanalysis

ianmkenney · 2020-07-26T02:07:20Z

Used PDBParser as a reference

Addresses #1387

Changes made in this Pull Request:

Repeated residues are not squashed
Multiple segids are created when multiple chains are present

PR Checklist

Tests?
Docs?
CHANGELOG updated?
Issue raised/referenced?

Used PDBParser as a reference

pep8speaks · 2020-07-26T02:07:23Z

Hello @ianmkenney! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-07-28 23:01:55 UTC

Should keep using segids to avoid confusion

codecov · 2020-07-26T08:19:13Z

Codecov Report

Merging #2872 into develop will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #2872      +/-   ##
===========================================
- Coverage    92.80%   92.78%   -0.02%     
===========================================
  Files          185      185              
  Lines        24205    24214       +9     
  Branches      3133     3137       +4     
===========================================
+ Hits         22463    22467       +4     
- Misses        1696     1701       +5     
  Partials        46       46

Impacted Files	Coverage Δ
package/MDAnalysis/topology/DMSParser.py	`90.54% <100.00%> (+1.65%)`	⬆️
coordinates/base.py	`94.19% <0.00%> (-0.63%)`	⬇️
util.py	`89.42% <0.00%> (-0.02%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2550d06...28dd7b7. Read the comment docs.

richardjgowers

hey @ianmkenney looks good but needs a simple test

ianmkenney · 2020-07-26T21:11:00Z

@richardjgowers I'm creating an altered ADK dms (adk_closed_domains.dms) from the already existing adk_closed.dms. Domain definitions are from:

Seyler SL, Kumar A, Thorpe MF, Beckstein O (2015) Path Similarity Analysis: A Method for Quantifying Macromolecular Pathways. PLoS Comput Biol 11(10): e1004568. https://doi.org/10.1371/journal.pcbi.1004568

Setting the domains as follows from sqlite3:

UPDATE particle SET segid = "CORE";

UPDATE particle 
SET segid = "NMP" 
WHERE resid BETWEEN 30 AND 59;

UPDATE particle 
SET segid = "LID" 
WHERE resid BETWEEN 122 AND 159;

A check on the selections of those domains should be sufficient for tests, right?

["A", "B", "A"] -> ["A", "B"]

Derived from adk_closed.dms and edited in sqlite3 UPDATE particle SET segid = "CORE"; UPDATE particle SET segid = "NMP" WHERE resid BETWEEN 30 AND 59; UPDATE particle SET segid = "LID" WHERE resid BETWEEN 122 AND 159; where the domain definitions can be found in: Seyler SL, Kumar A, Thorpe MF, Beckstein O (2015) Path Similarity Analysis: A Method for Quantifying Macromolecular Pathways. PLoS Comput Biol 11(10): e1004568. https://doi.org/10.1371/journal.pcbi.1004568

Added new selections in test_dms.py and relabeled previous selections

ianmkenney · 2020-07-27T01:26:25Z

@richardjgowers should be ready for a review

orbeckst

See comments and could you please also look at the PEP8 issues?

EDIT: Please also update CHANGELOG with a Fix entry and include issue #1387 in the message.

orbeckst · 2020-07-28T08:50:19Z

package/MDAnalysis/topology/DMSParser.py

                    vals = cur.fetchall()
                except sqlite3.DatabaseError:
-                    errmsg = "Failed reading the atoms from DMS Database"
-                    raise IOError(errmsg) from None


What was the reason for removing raise from None? See PR #2357 for rationale.

orbeckst · 2020-07-28T08:50:31Z

package/MDAnalysis/topology/DMSParser.py

                bonds = cur.fetchall()
            except sqlite3.DatabaseError:
-                errmsg = "Failed reading the bonds from DMS Database"
-                raise IOError(errmsg) from None


What was the reason for removing raise from None?

richardjgowers

I'm happy once @orbeckst 's question is answered, thanks @ianmkenney !

orbeckst

Looks good. I have one potential performance comment (see inline).

I changed your CHANGELOG entry to be under 2.0.0.

package/MDAnalysis/topology/DMSParser.py

orbeckst · 2020-07-29T01:05:49Z

Thanks @ianmkenney !

* partially addresses MDAnalysis#1387 for the DMS parser * Changes made in this Pull Request: * Repeated residues are not squashed * Multiple segids are created when multiple chains are present * Implemented extraction of chains from DMS formats * Properly split repeating segids: ["A", "B", "A"] -> ["A", "B"] * tests: New DMS file that includes multiple segids Derived from adk_closed.dms and edited in sqlite3 ```sql UPDATE particle SET segid = "CORE"; UPDATE particle SET segid = "NMP" WHERE resid BETWEEN 30 AND 59; UPDATE particle SET segid = "LID" WHERE resid BETWEEN 122 AND 159; ``` where the domain definitions can be found in: Seyler SL, Kumar A, Thorpe MF, Beckstein O (2015) Path Similarity Analysis: A Method for Quantifying Macromolecular Pathways. PLoS Comput Biol 11(10): e1004568. https://doi.org/10.1371/journal.pcbi.1004568 * tests: - Modified atom selections in test_dms.py - added new selections in test_dms.py and relabeled previous selections - Added DMS with no segid * Updated CHANGELOG

* partially addresses #1387 for the DMS parser * Changes made in this Pull Request: * Repeated residues are not squashed * Multiple segids are created when multiple chains are present * Implemented extraction of chains from DMS formats * Properly split repeating segids: ["A", "B", "A"] -> ["A", "B"] * tests: New DMS file that includes multiple segids Derived from adk_closed.dms and edited in sqlite3 ```sql UPDATE particle SET segid = "CORE"; UPDATE particle SET segid = "NMP" WHERE resid BETWEEN 30 AND 59; UPDATE particle SET segid = "LID" WHERE resid BETWEEN 122 AND 159; ``` where the domain definitions can be found in: Seyler SL, Kumar A, Thorpe MF, Beckstein O (2015) Path Similarity Analysis: A Method for Quantifying Macromolecular Pathways. PLoS Comput Biol 11(10): e1004568. https://doi.org/10.1371/journal.pcbi.1004568 * tests: - Modified atom selections in test_dms.py - added new selections in test_dms.py and relabeled previous selections - Added DMS with no segid * Updated CHANGELOG (cherry picked from commit 2f16a16)

* partially addresses MDAnalysis#1387 for the DMS parser * Changes made in this Pull Request: * Repeated residues are not squashed * Multiple segids are created when multiple chains are present * Implemented extraction of chains from DMS formats * Properly split repeating segids: ["A", "B", "A"] -> ["A", "B"] * tests: New DMS file that includes multiple segids Derived from adk_closed.dms and edited in sqlite3 ```sql UPDATE particle SET segid = "CORE"; UPDATE particle SET segid = "NMP" WHERE resid BETWEEN 30 AND 59; UPDATE particle SET segid = "LID" WHERE resid BETWEEN 122 AND 159; ``` where the domain definitions can be found in: Seyler SL, Kumar A, Thorpe MF, Beckstein O (2015) Path Similarity Analysis: A Method for Quantifying Macromolecular Pathways. PLoS Comput Biol 11(10): e1004568. https://doi.org/10.1371/journal.pcbi.1004568 * tests: - Modified atom selections in test_dms.py - added new selections in test_dms.py and relabeled previous selections - Added DMS with no segid * Updated CHANGELOG

Implemented extraction of chains from DMS formats

47461c9

Used PDBParser as a reference

ianmkenney added 2 commits July 25, 2020 22:27

Accidentally removed bond information

1df6d91

DMS databases contain chains and segids

5cfb0f7

Should keep using segids to avoid confusion

richardjgowers requested changes Jul 26, 2020

View reviewed changes

ianmkenney added 4 commits July 26, 2020 15:53

Properly split repeating segids

e0b6aef

["A", "B", "A"] -> ["A", "B"]

Modified atom selections in test_dms.py

92f367d

Added new selections in test_dms.py and relabeled previous selections

Added DMS_DOMAINS to __all__

5769e97

ianmkenney added 2 commits July 27, 2020 14:07

Case where segids are empty strings

f89e5b6

Added DMS with no segid

67dc66a

orbeckst added the Format-Desmond Desmond and Anton (DMS, STK) label Jul 28, 2020

orbeckst linked an issue Jul 28, 2020 that may be closed by this pull request

Multiple segments reduce to one segid #1387

Open

orbeckst removed a link to an issue Jul 28, 2020

Multiple segments reduce to one segid #1387

Open

orbeckst reviewed Jul 28, 2020

View reviewed changes

richardjgowers approved these changes Jul 28, 2020

View reviewed changes

ianmkenney added 3 commits July 28, 2020 11:27

Updated CHANGELOG

3525e5f

PEP8 fixes and error handling improvements

e9b1c45

PEP8 fixes

23af485

orbeckst self-assigned this Jul 28, 2020

orbeckst added this to the 1.0.x milestone Jul 28, 2020

moved CHANGELOG entry to 2.0.0

28dd7b7

orbeckst approved these changes Jul 28, 2020

View reviewed changes

package/MDAnalysis/topology/DMSParser.py Show resolved Hide resolved

orbeckst merged commit 2f16a16 into MDAnalysis:develop Jul 29, 2020

orbeckst mentioned this pull request Jul 29, 2020

Backport 1.0.1 #2798

Merged

4 tasks

fiona-naughton added Component-Topology defect labels Sep 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented extraction of chains from DMS formats#2872

Implemented extraction of chains from DMS formats#2872
orbeckst merged 13 commits intoMDAnalysis:developfrom
ianmkenney:fix/dms-chains

ianmkenney commented Jul 26, 2020 •

edited by orbeckst

Loading

Uh oh!

pep8speaks commented Jul 26, 2020 •

edited

Loading

Uh oh!

codecov bot commented Jul 26, 2020 •

edited

Loading

Uh oh!

richardjgowers left a comment

Uh oh!

ianmkenney commented Jul 26, 2020

Uh oh!

ianmkenney commented Jul 27, 2020

Uh oh!

orbeckst left a comment •

edited

Loading

Uh oh!

orbeckst Jul 28, 2020

Uh oh!

orbeckst Jul 28, 2020

Uh oh!

richardjgowers left a comment

Uh oh!

orbeckst left a comment

Uh oh!

Uh oh!

orbeckst commented Jul 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

ianmkenney commented Jul 26, 2020 • edited by orbeckst Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

pep8speaks commented Jul 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2020-07-28 23:01:55 UTC

Uh oh!

codecov bot commented Jul 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

richardjgowers left a comment

Choose a reason for hiding this comment

Uh oh!

ianmkenney commented Jul 26, 2020

Uh oh!

ianmkenney commented Jul 27, 2020

Uh oh!

orbeckst left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

orbeckst Jul 28, 2020

Choose a reason for hiding this comment

Uh oh!

orbeckst Jul 28, 2020

Choose a reason for hiding this comment

Uh oh!

richardjgowers left a comment

Choose a reason for hiding this comment

Uh oh!

orbeckst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

orbeckst commented Jul 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ianmkenney commented Jul 26, 2020 •

edited by orbeckst

Loading

pep8speaks commented Jul 26, 2020 •

edited

Loading

codecov bot commented Jul 26, 2020 •

edited

Loading

orbeckst left a comment •

edited

Loading