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

Process the tail text only after child elements (#333) #520

Merged
merged 2 commits into from
Oct 5, 2020

Conversation

HiromuHota
Copy link
Contributor

@HiromuHota HiromuHota commented Sep 30, 2020

Description of the problems or issues

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

See #333

Does your pull request fix any issue.

Fix #333

Description of the proposed changes

Process the tail text only after child elements

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.

@codecov-commenter
Copy link

codecov-commenter commented Sep 30, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #520      +/-   ##
==========================================
+ Coverage   85.81%   85.84%   +0.03%     
==========================================
  Files          90       90              
  Lines        4582     4593      +11     
  Branches      852      857       +5     
==========================================
+ Hits         3932     3943      +11     
  Misses        467      467              
  Partials      183      183              
Flag Coverage Δ
#unittests 85.84% <100.00%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
src/fonduer/parser/parser.py 93.39% <100.00%> (+0.23%) ⬆️

Comment on lines +119 to +146
# Check if the tail text is processed after inner elements (#333)
assert [sent.text for sent in doc.sentences[14:18]] == [
"italics and later",
"bold",
".",
"Even",
]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes sure that tail text is parsed after child elements.

@HiromuHota HiromuHota marked this pull request as ready for review September 30, 2020 21:53
@HiromuHota HiromuHota requested a review from senwu October 1, 2020 16:15
@senwu senwu requested a review from lukehsiao October 3, 2020 00:05
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.

LGTM.

You'll need to resolve the conflict, though.

@HiromuHota
Copy link
Contributor Author

@lukehsiao thanks for your review. Please merge once the tests finish.

@lukehsiao lukehsiao merged commit 48eef2d into HazyResearch:master Oct 5, 2020
@HiromuHota HiromuHota deleted the fix/333 branch October 5, 2020 18:33
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.

Inner HTML elements are processed after the tail text
3 participants