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

[MRG] Codecov options + remove 2.7 #178

Merged
merged 6 commits into from
May 19, 2020

Conversation

ayushkarnawat
Copy link
Contributor

What does this PR do?

Instead of specifying the files to ignore within .coveragerc, I believe it is recommended to add those settings inside the codecov.yml file instead.

I also removed some unnecessary imports and lines from setup that were no longer used.

As a side note, for now, I have specified that the codecov bot only comments on pull requests whenever there is a change (current behavior), should we set it to comment even if there are no changes to the coverage (recommend by codecov)?

@codecov-io
Copy link

Codecov Report

Merging #178 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #178   +/-   ##
=======================================
  Coverage   92.21%   92.21%           
=======================================
  Files          15       15           
  Lines        3007     3007           
=======================================
  Hits         2773     2773           
  Misses        234      234           

@ayushkarnawat
Copy link
Contributor Author

Also, since we moved strictly away from python 2.7, should we remove the externals module since it is no longer necessary?

setup.py Outdated
'Topic :: Scientific/Engineering :: Artificial Intelligence',
'Topic :: Scientific/Engineering :: Mathematics',
'Topic :: Scientific/Engineering :: Information Analysis',
'Programming Language :: Python :: 2',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Python 2 should be removed

@agramfort
Copy link
Collaborator

Yes external should be removed

@ayushkarnawat ayushkarnawat changed the title Ignore certain files in Codecov Remove py2.7 files May 16, 2020
@ayushkarnawat
Copy link
Contributor Author

ayushkarnawat commented May 16, 2020

This is probably not the best place to address this, but I think, for PRs, we should always squash the changes rather than merge commit them. This prevents us from logging the whole git history (i.e. minor commits to fix issues addressed by reviewers). By squashing the changes into 1 commit, we only save the history of the changes that matter (i.e. change from the previous pull request). It leaves a much cleaner git history, in my opinion.

@agramfort
Copy link
Collaborator

agramfort commented May 16, 2020 via email

@ayushkarnawat
Copy link
Contributor Author

I think there may be some issues with the codecov tool, its not reporting on the coverage correctly. Working on a fix rn

@ayushkarnawat
Copy link
Contributor Author

ayushkarnawat commented May 17, 2020

It seems that the codecov reports failed to upload properly, I think this error is on codecov's end. See here for full build details. Hopefully, it works correctly on the next commit!

Error: 400 Client Error: Bad Request for url: https://storage.googleapis.com/codecov/v4/raw/2020-05-17/696E8FC9A239519C4D149C953501626B/7e105c15fa17192eac0d3985188fb282652da2d1/46f5ab3b-f76d-4562-b3c9-f31dbe2e6a09.txt?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=GOOG1EQX6OZVJGHKK3633AAFGLBUCOOATRACRQRQF6HMSMLYUP6EAD6XSWAAY%2F20200517%2FUS%2Fs3%2Faws4_request&X-Amz-Date=20200517T143224Z&X-Amz-Expires=10&X-Amz-SignedHeaders=host&X-Amz-Signature=a1384d3f15efc4d169d7344583b78d3037fa3dacaf6f21e48239f07bd5b3b811

@rflamary rflamary changed the title Remove py2.7 files Codecov options + remove 2.7 May 18, 2020
@rflamary rflamary changed the title Codecov options + remove 2.7 [MRG] Codecov options + remove 2.7 May 18, 2020
@rflamary rflamary merged commit ad04f50 into PythonOT:master May 19, 2020
@rflamary
Copy link
Collaborator

@ayushkarnawat actually you had the wrong path for gpu it is actually ot/gpu/*.py

Sorry i did not see that earlier. Right now we are at 88% caverage could you fix that please?

@ayushkarnawat
Copy link
Contributor Author

Whoops my bad. Sorry for the late reply. Seems like you already fixed it! Thanks for catching it.

@ayushkarnawat ayushkarnawat deleted the remove-files branch May 19, 2020 14:11
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.

4 participants