Skip to content

Make align AnalysisBase classes use the Results class#3280

Merged
IAlibay merged 4 commits intoMDAnalysis:developfrom
IAlibay:results-align
May 7, 2021
Merged

Make align AnalysisBase classes use the Results class#3280
IAlibay merged 4 commits intoMDAnalysis:developfrom
IAlibay:results-align

Conversation

@IAlibay
Copy link
Member

@IAlibay IAlibay commented May 7, 2021

Fixes #3278
Towards #3261

Changes made in this Pull Request:

  • align.AlignTraj.rmsd -> align.AlignTraj.results.rmsd
  • align.AverageStructure.universe ->align.AverageStructure.results.universe
  • align.AverageStructure.positions -> align.AverageStructure.results.positions
  • align.AverageStructure.rmsd -> align.AverageStructure.results.rmsd

PR Checklist

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

@pep8speaks
Copy link

pep8speaks commented May 7, 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-07 18:38:23 UTC

@codecov
Copy link

codecov bot commented May 7, 2021

Codecov Report

Merging #3280 (3429bd0) into develop (8deaa1e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3280   +/-   ##
========================================
  Coverage    93.03%   93.04%           
========================================
  Files          172      172           
  Lines        22732    22752   +20     
  Branches      3194     3194           
========================================
+ Hits         21149    21169   +20     
  Misses        1533     1533           
  Partials        50       50           
Impacted Files Coverage Δ
package/MDAnalysis/analysis/align.py 98.32% <100.00%> (+0.12%) ⬆️

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 8deaa1e...3429bd0. Read the comment docs.

@IAlibay IAlibay added this to the 2.0 milestone May 7, 2021
@orbeckst orbeckst requested a review from fiona-naughton May 7, 2021 18:32
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


self.filename = filename
self.universe = mda.Merge(self.mobile_atoms)
self.results.universe = mda.Merge(self.mobile_atoms)
Copy link
Member

Choose a reason for hiding this comment

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

FYI @joaomcteixeira @PicoCentauri : mdacli will need to be able to deal with whole Universes in the results. This can be as simple as reporting that this result cannot be meaningfully saved. (Although, you can serialize a Universe nowadays — just the question if you want to).

@IAlibay IAlibay merged commit f8b20fb into MDAnalysis:develop May 7, 2021
@IAlibay IAlibay deleted the results-align branch May 7, 2021 20:31
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 AlignTraj and AverageStructure to use new .results

3 participants