Skip to content

Switch BAT to use Results#3273

Merged
IAlibay merged 4 commits intoMDAnalysis:developfrom
IAlibay:results-bat
May 6, 2021
Merged

Switch BAT to use Results#3273
IAlibay merged 4 commits intoMDAnalysis:developfrom
IAlibay:results-bat

Conversation

@IAlibay
Copy link
Member

@IAlibay IAlibay commented May 6, 2021

Fixes #3272
Towards #3261

Changes made in this Pull Request:

  • BAT is new in 2.0.0, so it's a direct switch from bat to results.bat.
  • Did a bit of cleaning up whilst I was there, including; a misindented code block and added a test.

PR Checklist

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

@pep8speaks
Copy link

pep8speaks commented May 6, 2021

Hello @IAlibay! 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 2021-05-06 21:08:25 UTC

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, thanks

Comment on lines +177 to +178
* `bat.BAT` now uses the `analysis.base.Results` class to store the `bat`
results attribute (Issue #3261, #3272)
Copy link
Member

Choose a reason for hiding this comment

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

Technically not needed because bat wasn't released – but I think I said in the past "if it has an issue then list it"... so leave it in

Copy link
Member

Choose a reason for hiding this comment

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

I changed the issue text to include CHANGELOG entry

@codecov
Copy link

codecov bot commented May 6, 2021

Codecov Report

Merging #3273 (9e60359) into develop (2c714fc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3273   +/-   ##
========================================
  Coverage    93.02%   93.02%           
========================================
  Files          172      172           
  Lines        22703    22704    +1     
  Branches      3193     3193           
========================================
+ Hits         21119    21121    +2     
+ Misses        1534     1533    -1     
  Partials        50       50           
Impacted Files Coverage Δ
package/MDAnalysis/analysis/bat.py 97.57% <100.00%> (+0.62%) ⬆️

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 2c714fc...9e60359. Read the comment docs.

@IAlibay IAlibay merged commit 503cf1e into MDAnalysis:develop May 6, 2021
@IAlibay IAlibay deleted the results-bat branch May 6, 2021 21:50
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 analysis.bat.BAT to use .results

3 participants