MDAnalysis / pmda

Parallel algorithms for MDAnalysis
https://www.mdanalysis.org/pmda/
Other
31 stars 22 forks source link

Add cdf function to InterRDF #99

Closed VOD555 closed 4 years ago

VOD555 commented 5 years ago

Changes made in this Pull Request:

PR Checklist

codecov[bot] commented 5 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@3605ee8). Click here to learn what that means. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #99   +/-   ##
=========================================
  Coverage          ?   97.82%           
=========================================
  Files             ?       11           
  Lines             ?      644           
  Branches          ?       78           
=========================================
  Hits              ?      630           
  Misses            ?        8           
  Partials          ?        6
Impacted Files Coverage Δ
pmda/leaflet.py 91.3% <ø> (ø)
pmda/rdf.py 100% <100%> (ø)

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 3605ee8...5d3716a. Read the comment docs.

orbeckst commented 5 years ago

(And please fix PEP8...)

orbeckst commented 5 years ago

Please also address the remaining comments.

VOD555 commented 5 years ago

@orbeckst All required changes have been done. Please have a look.

orbeckst commented 4 years ago

@VOD555 sorry, the PR fell off my radar. All looking good... will merge once Travis shows all green.

orbeckst commented 4 years ago

The RDF tests actually failed. Not sure why. Can you please look into this, @VOD555 ?

Perhaps changes in MDAnalysis?

orbeckst commented 4 years ago

Other tests (on master https://travis-ci.org/MDAnalysis/pmda ...) also fail in RDF, so it's not a problem with your PR but the reference values are not correct anymore.

Please have a look and decide what to do: either adjust the references or raise an issue for the upstream change that breaks our tests here.

orbeckst commented 4 years ago

@VOD555 I opened #110 for the RDF failure.

orbeckst commented 4 years ago

Hopefully the lint test passes now, too.

orbeckst commented 4 years ago

I am rebasing to master and squashing commits locally. I will force push this branch with the rewritten history

orbeckst commented 4 years ago

once this passes, do a simple merge