Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Cython Coverage #2255

Merged
merged 4 commits into from
Apr 22, 2019
Merged

Conversation

fenilsuchak
Copy link
Member

@fenilsuchak fenilsuchak commented Apr 18, 2019

Changes made in this Pull Request:

  • Sorry, I messed up some commits in the last one. Anyway, even by forcing cython_linetrace = True the codecov report doesn't show coverage for .pyx files. Locally works perfectly with pytest-cov. The sample html output is here. Any suggestions?

image

PR Checklist

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

@codecov
Copy link

codecov bot commented Apr 18, 2019

Codecov Report

Merging #2255 into develop will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2255      +/-   ##
===========================================
- Coverage     89.7%   89.65%   -0.05%     
===========================================
  Files          160      172      +12     
  Lines        19769    21396    +1627     
  Branches      2783     2783              
===========================================
+ Hits         17733    19183    +1450     
- Misses        1440     1616     +176     
- Partials       596      597       +1
Impacted Files Coverage Δ
package/MDAnalysis/lib/transformations.py 78.47% <0%> (-0.15%) ⬇️
package/MDAnalysis/analysis/encore/cutils.pyx 100% <0%> (ø)
package/MDAnalysis/lib/formats/libmdaxdr.pyx 80.9% <0%> (ø)
package/MDAnalysis/lib/c_distances.pyx 100% <0%> (ø)
package/MDAnalysis/lib/qcprot.pyx 100% <0%> (ø)
...alysis/analysis/encore/clustering/affinityprop.pyx 90.24% <0%> (ø)
package/MDAnalysis/lib/formats/cython_util.pyx 90% <0%> (ø)
package/MDAnalysis/lib/formats/libdcd.pyx 79.28% <0%> (ø)
package/MDAnalysis/lib/_cutil.pyx 98.81% <0%> (ø)
...e/dimensionality_reduction/stochasticproxembed.pyx 100% <0%> (ø)
... and 4 more

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 fdeb724...a8d05b7. Read the comment docs.

@fenilsuchak
Copy link
Member Author

Ok, probably I've traced the issue. Keeping the current changes as in this commit and building locally in editable mode with pip install -e package cython files are covered with coverage report visible but when I build it via pip install -v package as done by travis, the cython files aren't traced. Not sure of the exact reason why, but the search has been narrowed.

@richardjgowers
Copy link
Member

richardjgowers commented Apr 19, 2019 via email

@fenilsuchak
Copy link
Member Author

Yuhu! .pyx covered in report!!

@fenilsuchak
Copy link
Member Author

Travis failing due to doc in the _augment.pyx cython file. Not sure how to fix it. Any help would be appreciated

@zemanj
Copy link
Member

zemanj commented Apr 20, 2019

@Fenil3510 Sphinx doesn't play nicely with Cython if embedsignature=True. Make sure to set embedsignature=False in the doc build and everything should work.

@fenilsuchak
Copy link
Member Author

I believe codecov is failing due to low coverage of Cython files

@zemanj
Copy link
Member

zemanj commented Apr 20, 2019 via email

@richardjgowers
Copy link
Member

richardjgowers commented Apr 21, 2019

Screen Shot 2019-04-21 at 9 49 18 AM

Something strange is happening where we're getting the repo twice now...

e: seems like this isn't caused by this PR

Copy link
Member

@richardjgowers richardjgowers left a comment

Choose a reason for hiding this comment

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

Ok looks good, +12 files in coverage which seems about right

@richardjgowers richardjgowers merged commit c7c394e into MDAnalysis:develop Apr 22, 2019
@fenilsuchak fenilsuchak mentioned this pull request Apr 23, 2019
4 tasks
orbeckst added a commit that referenced this pull request Dec 21, 2023
Update of AUTHORS and CHANGELOG with inferred author contributions.

*  Removed duplicate mattwthompson in 0.20.0 changelog entry.: mattwthompson was placed twice by accident, this removes this duplication.

* Addition of missing authors.

   An retrospective analysis of the authors found via `git shortlog -s -n --email --branches="develop"` found several commits by authors which were not present in the `AUTHORS.md` file.

    - Zhenbo Li: commited via PR: Started tests for gnm. #803 and Make Travis run tests on OSX. #771,
    - Jenna M. Swarthout via PR Update CoC according to suggestions from current CoC committee #4289 and Point to new reporting form link (owned by conduct@mdanalysis.org) #4298,
    - Bradley Dice via PR   Fix GSD Windows compatibility #2384 ,
    - David Minh via PR #2668

   There seemed to be no indications in the above mentioned PRs that the author did not want to be in the authors file, it looks like it was just forgotten.

* Addition of missing entries from the changelog

   Continued cross referencing of the git shortlog output but also accounting for the changelog identified several individuals that had not been included in the changelog entries for the release they contributed under. They were added to the relevant entry of the changelog based on the merge commit date.

   Please note that for Tone Bengsten, we a) had no github handle (so they were assigned @tbengtsen), and b) no specific commit. Given we know that this individual was including alongside the encore merge, they were assigned to the 0.16.0 release.

* Update CHANGELOG
* PR #1218
* PR #1284 and #1408
* PR #4109
* based on git history
* PRs #803 and #771 (author addition, changelog addition)
* PR #2255 and #2221
* PR #1225
* PR #4289 and #4298
* PR #4031
* PR #4085
* PR #3635
* PR #2356
* PR #2559
* No GH handle - Encore author addition @tbengtsen
* PR #4184
* PR #2614
* PR #2219
* PR #2384
* PR #2668
* Add missing entry for Jenna

Thanks to @fiona-naughton for helping out with digging into this data :)

Co-authored-by: Fiona Naughton <fiona@mdanalysis.org>
Co-authored-by: Oliver Beckstein <orbeckst@mdanalysis.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants