Skip to content

Switch hbonds to results.hbonds for HydrogenBondAnalysis#3283

Merged
orbeckst merged 1 commit intoMDAnalysis:developfrom
IAlibay:results-hbond
May 7, 2021
Merged

Switch hbonds to results.hbonds for HydrogenBondAnalysis#3283
orbeckst merged 1 commit intoMDAnalysis:developfrom
IAlibay:results-hbond

Conversation

@IAlibay
Copy link
Member

@IAlibay IAlibay commented May 7, 2021

Fixes #3271
Towards #3261

Changes made in this Pull Request:

  • Switch hbonds to results.hbonds for HydrogenBondAnalysis
  • Some docstring changes

PR Checklist

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

@pep8speaks
Copy link

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

Line 738:80: E501 line too long (81 > 79 characters)

@codecov
Copy link

codecov bot commented May 7, 2021

Codecov Report

Merging #3283 (9e11f7f) into develop (da46e84) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3283   +/-   ##
========================================
  Coverage    93.03%   93.03%           
========================================
  Files          172      172           
  Lines        22724    22731    +7     
  Branches      3193     3193           
========================================
+ Hits         21141    21148    +7     
  Misses        1533     1533           
  Partials        50       50           
Impacted Files Coverage Δ
...DAnalysis/analysis/hydrogenbonds/hbond_analysis.py 98.70% <100.00%> (+0.06%) ⬆️

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 da46e84...9e11f7f. Read the comment docs.

Copy link
Member

@p-j-smith p-j-smith left a comment

Choose a reason for hiding this comment

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

It looks good to me, thanks @IAlibay !

@orbeckst orbeckst self-assigned this May 7, 2021
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.

lgtm

Thank you @p-j-smith and @bieniekmateusz for reviewing at short notice. Much appreciated!!

self.d_h_a_angle = d_h_a_angle_cutoff
self.update_selections = update_selections
self.hbonds = None
self.results = Results()
Copy link
Member

Choose a reason for hiding this comment

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

Ok, that's necessary when not calling super().__init__()

@orbeckst orbeckst merged commit 8deaa1e into MDAnalysis:develop May 7, 2021
@orbeckst
Copy link
Member

orbeckst commented May 7, 2021

one down, thank you @IAlibay

@orbeckst orbeckst added this to the 2.0 milestone May 7, 2021
@IAlibay IAlibay deleted the results-hbond branch January 30, 2022 18:53
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.

update hydrogenbonds.HydrogenBondAnalysis to use .results

5 participants