8000 [MRG+1] activate coverage also with python 2.7 by aabadie · Pull Request #493 · joblib/joblib · GitHub
[go: up one dir, main page]
Skip to content

[MRG+1] activate coverage also with python 2.7 #493

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Feb 7, 2017
Merged

[MRG+1] activate coverage also with python 2.7 #493

merged 7 commits into from
Feb 7, 2017

Conversation

aabadie
Copy link
Contributor
@aabadie aabadie commented Feb 6, 2017 •
edited
Loading

Testing if codecov merges the coverages generated with the 2 builds (python 2.7 and python 3.5 3.6).

@codecov-io
Copy link
codecov-io commented Feb 6, 2017 •
edited by codecov bot
Loading

Codecov Report

Merging #493 into master will increase coverage by 3.78%.

@@            Coverage Diff             @@
##           master     #493      +/-   ##
==========================================
+ Coverage   90.16%   93.94%   +3.78%     
==========================================
  Files          36       36              
  Lines        4575     4575              
==========================================
+ Hits         4125     4298     +173     
+ Misses        450      277     -173
Impacted Files Coverage Δ
joblib/format_stack.py 83.25% <ø> (+0.47%) ✅
joblib/test/test_hashing.py 98.88% <ø> (+0.55%) ✅
joblib/parallel.py 97.03% <ø> (+1.11%) ✅
joblib/logger.py 85.52% <ø> (+1.31%) ✅
joblib/_parallel_backends.py 91.21% <ø> (+1.35%) ✅
joblib/memory.py 93.25% <ø> (+1.5%) ✅
joblib/numpy_pickle_compat.py 91% <ø> (+2%) ✅
joblib/test/test_func_inspect.py 89.79% <ø> (+2.04%) ✅
joblib/test/test_pool.py 98.78% <ø> (+2.43%) ✅
joblib/numpy_pickle_utils.py 93.29% <ø> (+3.51%) ✅
... and 12 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 919f69e...a8c8dee. Read the comment docs.

@aabadie
Copy link
Contributor Author
aabadie commented Feb 6, 2017

It indeed merges the results and thus looks much better :)

@aabadie aabadie changed the title activate coverage also with python 2.7 [MRG] activate coverage also with python 2.7 Feb 6, 2017
@GaelVaroquaux GaelVaroquaux changed the title [MRG] activate coverage also with python 2.7 [MRG+1] activate coverage also with python 2.7 Feb 6, 2017
@GaelVaroquaux
Copy link
Member

Yes, it's pretty neat! Good job!

+1 for merge.

@GaelVaroquaux GaelVaroquaux mentioned this pull request Feb 6, 2017
Closed
@aabadie
Copy link
Contributor Author
aabadie commented Feb 6, 2017

Also run coverage with the build that doesn't have numpy will increase again coverage (lots of ImportError catches covered), I just pushed a commit with this change.

@aabadie
Copy link
Contributor Author
aabadie commented Feb 6, 2017

I feel really enthusiastic with codecov : looking at the coverage reports, I can find things that could be improved (useless code, untested but testable parts, etc). It's a great tool to produce better code !

@lesteve
Copy link
Member
lesteve commented Feb 6, 2017

You probably want to have a codecov.yml to disable comments, like they do in loky.

@aabadie
Copy link
Contributor Author
aabadie commented Feb 6, 2017

You probably want to have a codecov.yml to disable comments

Thanks for the tip, I added that.

@lesteve
Copy link
Member
lesteve commented Feb 6, 2017

Thanks for the tip, I added that.

Actually I read the docs in more details and it looks like codecov only posts one comment and then edits it on each further commit: https://docs.codecov.io/docs/pull-request-comments#section-behavior.

This means that it causes only one email rather than spamming on each commit. My take on it is that the report is quite useful, so I would be in favour of removing codecov.yml. Sorry for changing my mind.

@lesteve
Copy link
Member
lesteve commented Feb 7, 2017

Enabling the coverage on Windows jumps to mind as well. Can be done in a separate PR.

@aabadie
Revert "disable comments"
f93f8be 
This reverts commit 6207f7e.
@lesteve
Copy link
Member
lesteve commented Feb 7, 2017

Can you add the COVERAGE for the JOBLIB_MULTIPROCESSING=0 as well?

@aabadie
Copy link
Contributor Author
aabadie commented Feb 7, 2017

I pushed a commit for appveyor but paths are changed during the run and codecov can't find the coverage file. I'll try that in a separate PR.
It should be good to go now.

@lesteve
Copy link
Member
lesteve commented Feb 7, 2017

Great stuff, thanks, merging!

@lesteve lesteve merged commit c60f943 into joblib:master Feb 7, 2017
yarikoptic added a commit to yarikoptic/joblib that referenced this pull request Nov 3, 2017
@yarikoptic
Merge tag '0.11' into releases
0d2e765 
* tag '0.11': (60 commits)
  Add configuration for readthedocs
  Minor fixes in CHANGES.rst
  Set relase version to 0.11
  Add 0.11 section to changelog
  Fix rst for correct rendering on PyPI
  MAINT update release instructions to use twine
  [MRG] Fix AppVeyor test exit code (joblib#505)
  [MRG] turn warnings into error in the doc generation (joblib#504)
  MAINT replace py.test by pytest
  MAINT remove most warnings from documentating the doc
  Shorter error message when cache results can not be loaded. (joblib#501)
  Replace deprecated logging.warn by logging.warning
  add pypi badge to readme and move the bagdes to the top (joblib#500)
  Enable codecov on AppVeyor (joblib#497)
  Improve folder organisation for CI files (joblib#499)
  Revert flake8 version to 2.5.1
  Use Python 3.5 on AppVeyor (joblib#495)
  Fix race condition in Memory when persisting output in parallel (joblib#491)
  WindowsError should be a type under non-Windows platforms (joblib#494)
  [MRG+1] activate coverage on multiple builds (joblib#493)
  ...
yarikoptic added a commit to yarikoptic/joblib that referenced this pull request Nov 3, 2017
@yarikoptic
Merge commit '0.11-7-g0d2e765' into debian
646c34c 
* commit '0.11-7-g0d2e765': (60 commits)
  Add configuration for readthedocs
  Minor fixes in CHANGES.rst
  Set relase version to 0.11
  Add 0.11 section to changelog
  Fix rst for correct rendering on PyPI
  MAINT update release instructions to use twine
  [MRG] Fix AppVeyor test exit code (joblib#505)
  [MRG] turn warnings into error in the doc generation (joblib#504)
  MAINT replace py.test by pytest
  MAINT remove most warnings from documentating the doc
  Shorter error message when cache results can not be loaded. (joblib#501)
  Replace deprecated logging.warn by logging.warning
  add pypi badge to readme and move the bagdes to the top (joblib#500)
  Enable codecov on AppVeyor (joblib#497)
  Improve folder organisation for CI files (joblib#499)
  Revert flake8 version to 2.5.1
  Use Python 3.5 on AppVeyor (joblib#495)
  Fix race condition in Memory when persisting output in parallel (joblib#491)
  WindowsError should be a type under non-Windows platforms (joblib#494)
  [MRG+1] activate coverage on multiple builds (joblib#493)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aabadie @codecov-io @GaelVaroquaux @lesteve
0