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

enabled coverage for cython #2029

Closed
wants to merge 4 commits into from
Closed

Conversation

richardjgowers
Copy link
Member

Fixes #443

Changes made in this Pull Request:

PR Checklist

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

@tylerjereddy
Copy link
Member

I think @stefanv and the scikit-image team are quite interested in doing this for their library as well so cc'ing them here.

@richardjgowers
Copy link
Member Author

@tylerjereddy I'm just seeing if it's as easy as these two lines... if it is I'll bring them in :)

@seb-buch
Copy link
Contributor

seb-buch commented Aug 6, 2018

@richardjgowers it basically is!.. with one caveat: lines with "cdef" are marked as "not covered" even if they actually are. This is a well known bug/issue (see cython/cython#1461).
Even with "embedsignature" set to true, I still have "false negative" for basically all definition lines.

@stefanv
Copy link

stefanv commented Aug 6, 2018

/cc @jni @soupault

@codecov
Copy link

codecov bot commented Aug 7, 2018

Codecov Report

Merging #2029 into develop will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2029      +/-   ##
===========================================
+ Coverage    88.59%   88.59%   +<.01%     
===========================================
  Files          143      143              
  Lines        17305    17361      +56     
  Branches      2649     2658       +9     
===========================================
+ Hits         15331    15381      +50     
- Misses        1376     1379       +3     
- Partials       598      601       +3
Impacted Files Coverage Δ
package/MDAnalysis/coordinates/memory.py 94.28% <0%> (-0.11%) ⬇️
package/MDAnalysis/lib/__init__.py 100% <0%> (ø) ⬆️
package/MDAnalysis/lib/distances.py 87.88% <0%> (+0.24%) ⬆️

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 10e16c0...a340397. Read the comment docs.

@richardjgowers
Copy link
Member Author

Hmm this is weird. When I run locally, the ascii table printout doesn't have any .pyx files, but if I look inside the .coverage file produced there are entries for .pyx files, so it's sort of working...

@orbeckst
Copy link
Member

orbeckst commented Apr 8, 2019

@Fenil3510 for your GSoC project proposal "Cythonizing ASCII Readers/Parser", the discussion here should be relevant.

@fenilsuchak
Copy link
Member

I was researching on adding cython coverage, the codecov report here still doesn't track .pyx files. I ran the same locally with some minor changes(hacky) and the report covers .pyx files

Here's the HTML output
image

I have opened a separate PR for it as I will be working on this issue for my project.

@fenilsuchak
Copy link
Member

This PR can be closed as #2255 solves it.

@richardjgowers richardjgowers deleted the issue-443-cythoncoverage branch April 23, 2019 09:36
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.

add cython code to coverage report
6 participants