Navigation Menu

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

Cython coverage #49

Merged
merged 10 commits into from Jun 18, 2020
Merged

Cython coverage #49

merged 10 commits into from Jun 18, 2020

Conversation

saimn
Copy link
Contributor

@saimn saimn commented Jun 18, 2020

Fix #30.

To get the Cython coverage, we need a "develop" build.
Also I moved the linetrace and profile directives, as well as definition of CYTHON_TRACE=1, to be optional since it can have a performance impact for normal builds. So this is activated in the setup.py by setting the COVERAGE env var.

Refs:
https://medium.com/@dfdeshom/better-test-coverage-workflow-for-cython-modules-631615eb197a
http://blog.behnel.de/posts/coverage-analysis-for-cython-modules.html

@codecov
Copy link

codecov bot commented Jun 18, 2020

Codecov Report

Merging #49 into master will decrease coverage by 5.11%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##            master      #49      +/-   ##
===========================================
- Coverage   100.00%   94.88%   -5.12%     
===========================================
  Files            2        7       +5     
  Lines            9     1017    +1008     
  Branches         0       53      +53     
===========================================
+ Hits             9      965     +956     
- Misses           0       52      +52     
Impacted Files Coverage Δ
astroscrappy/astroscrappy.pyx 75.11% <ø> (ø)
astroscrappy/utils/image_utils.pyx 100.00% <ø> (ø)
astroscrappy/utils/median_utils.pyx 100.00% <ø> (ø)
astroscrappy/utils/medutils.c 100.00% <0.00%> (ø)
astroscrappy/utils/imutils.c 100.00% <0.00%> (ø)

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 ca9f46c...1e6331e. Read the comment docs.

@saimn
Copy link
Contributor Author

saimn commented Jun 18, 2020

@cmccully
Copy link
Contributor

Wow! Awesome!

@cmccully
Copy link
Contributor

Can we add the c files somehow?

@saimn
Copy link
Contributor Author

saimn commented Jun 18, 2020

In theory yes, with CFLAGS=--coverage, and codecov is running gcov by default, but I was getting a undefined symbol: __gcov_merge_add error. But now I think I know why, let's try it again.

@saimn
Copy link
Contributor Author

saimn commented Jun 18, 2020

Yes, it works (I was defining CFLAGS in two places, that wasn't good :))
https://codecov.io/gh/astropy/astroscrappy/tree/84c0e9e4ceb0028ab2437dfe34aa441eb06f5326/astroscrappy/utils

@cmccully
Copy link
Contributor

That's awesome!!

@cmccully
Copy link
Contributor

This looks great. Are we ready to merge?

@saimn
Copy link
Contributor Author

saimn commented Jun 18, 2020

Happy to see this working, it will be useful for other projects :)

@saimn
Copy link
Contributor Author

saimn commented Jun 18, 2020

Yep, it's good to go!

@cmccully cmccully merged commit fd4acfe into astropy:master Jun 18, 2020
@saimn saimn deleted the cython-coverage branch June 18, 2020 21:37
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.

Coverage report is incorrect
2 participants