Skip to content

Conversation

HiromuHota
Copy link
Contributor

Fix #425.

@codecov-commenter
Copy link

codecov-commenter commented May 22, 2020

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.59%. Comparing base (af6947b) to head (7bb8662).
Report is 170 commits behind head on master.

Files with missing lines Patch % Lines
src/fonduer/utils/data_model_utils/visual.py 80.00% 0 Missing and 1 partial ⚠️

❌ Your project status has failed because the head coverage (82.59%) is below the target coverage (85.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #426      +/-   ##
==========================================
+ Coverage   82.47%   82.59%   +0.12%     
==========================================
  Files          86       86              
  Lines        4364     4366       +2     
  Branches      809      810       +1     
==========================================
+ Hits         3599     3606       +7     
+ Misses        575      572       -3     
+ Partials      190      188       -2     
Flag Coverage Δ
unittests 82.59% <80.00%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/fonduer/utils/data_model_utils/visual.py 88.23% <80.00%> (+3.90%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@HiromuHota
Copy link
Contributor Author

HiromuHota commented May 22, 2020

from_sentence looks erroneous. The description (document) and the behaviour (implementation) do not seem to match.
from_sentence was introduced by ff42e41, where from_sentence was called from_phrase.

@lukehsiao and @senwu , what do you think?

@HiromuHota
Copy link
Contributor Author

I figured that out. from_sentence does what it is supposed to do. It was just confusing. I'll add comments and improve the documentation.

@HiromuHota HiromuHota marked this pull request as ready for review May 22, 2020 22:26
@lukehsiao
Copy link
Contributor

@HiromuHota could you squash these commits with a nice commit description, then we can merge?

@HiromuHota
Copy link
Contributor Author

I added another unit test: test_get_ngrams_that_match_in_string (c55295d).
Sorry for the last minute commit!
If this looks good to you, I'll squash into one single commit.

@lukehsiao
Copy link
Contributor

Looks good to me!

…he input mention is not tabular.

Co-authored-by: Luke Hsiao <[email protected]>
@HiromuHota
Copy link
Contributor Author

Squashed and force-pushed.
Please merge this at your convenience.

@lukehsiao lukehsiao merged commit 014873c into HazyResearch:master May 27, 2020
@HiromuHota HiromuHota deleted the fix/425 branch May 27, 2020 03:51
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.

get_horz_ngrams works only when mention is_tabular
3 participants