Skip to content

Fixes AttributeError in HydrogenBondAnalysis #2849

Merged
orbeckst merged 5 commits intoMDAnalysis:developfrom
bieniekmateusz:hbonds_no_hydrogens
Jul 15, 2020
Merged

Fixes AttributeError in HydrogenBondAnalysis #2849
orbeckst merged 5 commits intoMDAnalysis:developfrom
bieniekmateusz:hbonds_no_hydrogens

Conversation

@p-j-smith
Copy link
Member

@p-j-smith p-j-smith commented Jul 14, 2020

Fixes #2848

Changes made in this Pull Request:

  • An attribute error is thrown when finding D-H pairs via the topology if hydrogens is an empty AtomGroup. This has been fixed so that an empty donors group is created, rather than assigning donors = 0.
  • Added a test case that raises the attribute without the fix.

PR Checklist

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

Co-authored-by: bieniekmateusz bieniekmat@gmail.com

Paul Smith and others added 3 commits July 14, 2020 17:21
An attribute error is thrown when finding D-H pairs via the topology is `hydrogens` is an empty AtomGroup.

Added a test case that raises the attribute without the fix.

Co-authored-by: bieniekmateusz <bieniekmat@gmail.com>
Co-authored-by: bieniekmateusz <bieniekmat@gmail.com>
@codecov
Copy link

codecov bot commented Jul 14, 2020

Codecov Report

Merging #2849 into develop will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2849      +/-   ##
===========================================
- Coverage    92.22%   92.20%   -0.03%     
===========================================
  Files          184      184              
  Lines        24170    24080      -90     
  Branches      3134     3115      -19     
===========================================
- Hits         22291    22203      -88     
+ Misses        1814     1812       -2     
  Partials        65       65              
Impacted Files Coverage Δ
...DAnalysis/analysis/hydrogenbonds/hbond_analysis.py 98.63% <100.00%> (+<0.01%) ⬆️
package/MDAnalysis/analysis/waterdynamics.py 95.58% <0.00%> (-0.56%) ⬇️

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 83f21dc...e173086. 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.

Looking good, just one minor CHANGELOG issue.

Can the same problem also happen for acceptors or is it restricted to the case of finding H via topology?

Paul Smith and others added 2 commits July 15, 2020 10:36
Rather than the top level alias.

Co-authored-by: bieniekmateusz <bieniekmat@gmail.com>
Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>
@orbeckst orbeckst self-assigned this Jul 15, 2020
@orbeckst
Copy link
Member

No sure why Travis CI currently states "cancelled" when the most recent check https://travis-ci.com/github/MDAnalysis/mdanalysis/builds/175684432 passed. I'll merge.

@orbeckst orbeckst merged commit 575c027 into MDAnalysis:develop Jul 15, 2020
orbeckst pushed a commit that referenced this pull request Jul 15, 2020
* Fix Issue #2848
* An attribute error is thrown when finding D-H pairs via the topology if hydrogens is an
   empty AtomGroup. This has been fixed so that an empty donors group is created, rather
   than assigning donors = 0.
* Added a test case that raises the attribute without the fix.
* Updated CHANGELOG
* add @bieniekmateusz and @p-j-smith to CHANGELOG 1.0.1 author section
  (was forgotten in PR #2849)

Co-authored-by: bieniekmateusz <bieniekmat@gmail.com>
(cherry picked from commit 575c027)
PicoCentauri pushed a commit to PicoCentauri/mdanalysis that referenced this pull request Mar 30, 2021
* Fix Issue MDAnalysis#2848
* An attribute error is thrown when finding D-H pairs via the topology if hydrogens is an 
   empty AtomGroup. This has been fixed so that an empty donors group is created, rather 
   than assigning donors = 0.
* Added a test case that raises the attribute without the fix.
* Updated CHANGELOG

Co-authored-by: bieniekmateusz <bieniekmat@gmail.com>
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.

HydrogenBondAnalysis AttributeError when empty hydrogens AtomGroup

3 participants