Skip to content

Conversation

HiromuHota
Copy link
Contributor

@HiromuHota HiromuHota commented Sep 9, 2020

Description of the problems or issues

Is your pull request related to a problem? Please describe.

See #503.

Does your pull request fix any issue.

Fix #503.

Description of the proposed changes

Copy textual functions in data_model_utils.tabular to data_model_utils.textual, and deprecated them in data_model_utils.tabular.

Test plan

A clear and concise description of how you test the new changes.

Checklist

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated the CHANGELOG.rst accordingly.

… to data_model_utils.textual

and deprecated them in data_model_utils.tabular (fix HazyResearch#503)
@HiromuHota HiromuHota marked this pull request as draft September 9, 2020 21:11
@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2020

Codecov Report

Merging #505 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #505      +/-   ##
==========================================
+ Coverage   85.80%   85.83%   +0.03%     
==========================================
  Files          88       88              
  Lines        4571     4583      +12     
  Branches      855      855              
==========================================
+ Hits         3922     3934      +12     
  Misses        464      464              
  Partials      185      185              
Flag Coverage Δ
#unittests 85.83% <100.00%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
setup.py 0.00% <ø> (ø)
src/fonduer/utils/data_model_utils/tabular.py 100.00% <100.00%> (ø)
src/fonduer/utils/data_model_utils/textual.py 72.09% <100.00%> (+14.95%) ⬆️

@HiromuHota HiromuHota changed the title Copy textual functions in data_model_utils.tabular to data_model_utils.textual Move textual functions in data_model_utils.tabular to data_model_utils.textual Sep 9, 2020
@HiromuHota HiromuHota marked this pull request as ready for review September 9, 2020 23:38
Copy link
Contributor

@lukehsiao lukehsiao left a comment

Choose a reason for hiding this comment

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

This reorg makes a lot of sense. Thanks for doing this, and for the nice deprecation warnings!

@lukehsiao lukehsiao merged commit b5765ab into HazyResearch:master Sep 11, 2020
@lukehsiao lukehsiao added this to the v0.8.3 milestone Sep 11, 2020
@lukehsiao lukehsiao added the clean-up Cleaning up the code or refactoring label Sep 11, 2020
@HiromuHota HiromuHota deleted the fix/503 branch September 11, 2020 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean-up Cleaning up the code or refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get_sentence_ngrams, get_neighbor_sentence_ngrams, same_sentence should be fonduer.utils.data_model_utils.textual?
3 participants