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] Fix issue 317 #318

Merged
merged 3 commits into from Dec 6, 2021
Merged

[MRG] Fix issue 317 #318

merged 3 commits into from Dec 6, 2021

Conversation

cshjin
Copy link
Contributor

@cshjin cshjin commented Dec 2, 2021

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Motivation and context / Related issue

To fix the issue #317

How has this been tested (if it applies)

pytest on test folders

Checklist

  • The documentation is up-to-date with the changes I made.
  • I have read the CONTRIBUTING document.
  • All tests passed, and additional code has been covered with new tests.

@codecov
Copy link

codecov bot commented Dec 2, 2021

Codecov Report

Merging #318 (4ac3fd5) into master (ca69658) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #318      +/-   ##
==========================================
+ Coverage   93.30%   93.34%   +0.04%     
==========================================
  Files          21       21              
  Lines        4901     4901              
==========================================
+ Hits         4573     4575       +2     
+ Misses        328      326       -2     

@rflamary rflamary changed the title Fix issue 317 [MRG] Fix issue 317 Dec 3, 2021
@rflamary rflamary changed the title [MRG] Fix issue 317 [WIP] Fix issue 317 Dec 3, 2021
@rflamary
Copy link
Collaborator

rflamary commented Dec 3, 2021

Hello @cshjin and thank you for the PR.

Could you also provide a test in foile test/test_gromov.py (that was failing previously) that ensures that the bug is not there anymore and prevent it from coming back?

@cshjin
Copy link
Contributor Author

cshjin commented Dec 3, 2021

@rflamary Thanks for your response. I add the test in the test_gromov.py and update docs associated with those two functions.

~/workspace/github/POT/test $ pytest test_gromov.py 
===================================================================================== test session starts =====================================================================================
platform linux -- Python 3.8.5, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: ~/workspace/github/POT, configfile: pytest.ini
collected 12 items                                                                                                                                                                            

test_gromov.py ............                                                                                                                                                             [100%]

====================================================================================== warnings summary =======================================================================================
# some warnings.

-- Docs: https://docs.pytest.org/en/stable/warnings.html
=============================================================================== 12 passed, 31 warnings in 9.65s ===============================================================================

@rflamary rflamary changed the title [WIP] Fix issue 317 [MRG] Fix issue 317 Dec 6, 2021
@rflamary rflamary merged commit b3dc68f into PythonOT:master Dec 6, 2021
@cshjin cshjin deleted the fix_issue_317 branch December 6, 2021 18: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.

None yet

2 participants