Skip to content

RDKitConverter bugfix on HIP residue#2941

Merged
orbeckst merged 2 commits intoMDAnalysis:developfrom
cbouy:fix-hip
Sep 18, 2020
Merged

RDKitConverter bugfix on HIP residue#2941
orbeckst merged 2 commits intoMDAnalysis:developfrom
cbouy:fix-hip

Conversation

@cbouy
Copy link
Member

@cbouy cbouy commented Sep 13, 2020

Currently HIP residues are likely to have a single double bond a a negatively charged carbon instead of 2 double bonds and a positive nitrogen

Changes made in this Pull Request:

  • Adds a new SMARTS reaction to fix the protonated histidine residue HIP

PR Checklist

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

@pep8speaks
Copy link

pep8speaks commented Sep 13, 2020

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

Line 606:80: E501 line too long (100 > 79 characters)
Line 608:80: E501 line too long (100 > 79 characters)
Line 610:80: E501 line too long (100 > 79 characters)
Line 612:80: E501 line too long (100 > 79 characters)
Line 614:80: E501 line too long (100 > 79 characters)
Line 616:80: E501 line too long (100 > 79 characters)
Line 618:80: E501 line too long (100 > 79 characters)
Line 620:80: E501 line too long (100 > 79 characters)
Line 622:80: E501 line too long (100 > 79 characters)
Line 624:80: E501 line too long (100 > 79 characters)
Line 626:80: E501 line too long (100 > 79 characters)
Line 628:1: W293 blank line contains whitespace

Comment last updated at 2020-09-15 18:04:16 UTC

@codecov
Copy link

codecov bot commented Sep 13, 2020

Codecov Report

Merging #2941 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2941   +/-   ##
========================================
  Coverage    93.02%   93.02%           
========================================
  Files          187      187           
  Lines        24962    24962           
  Branches      3261     3261           
========================================
  Hits         23220    23220           
  Misses        1694     1694           
  Partials        48       48           
Impacted Files Coverage Δ
package/MDAnalysis/coordinates/RDKit.py 97.31% <ø> (ø)

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 2c5e385...d1c1c49. Read the comment docs.

Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

Can you add a comment to the tests for the new SMILES that they are related to this fix?

(I don't think that this requires a CHANGELOG entry as it is updating an unreleased feature.)

@IAlibay @fiona-naughton anything else that you see?

@orbeckst
Copy link
Member

Thanks @cbouy – unless anyone else sees something obvious, anyone can merge it when convenient (just wait for CI).

@orbeckst orbeckst merged commit c4478d3 into MDAnalysis:develop Sep 18, 2020
PicoCentauri pushed a commit to PicoCentauri/mdanalysis that referenced this pull request Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants