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

Bugfixing and Refactoring parse.py #7

Merged
merged 7 commits into from
Jun 1, 2018
Merged

Bugfixing and Refactoring parse.py #7

merged 7 commits into from
Jun 1, 2018

Conversation

hayesall
Copy link
Member

This pull request re-factors the makeIdentifiers() function in parse.py and fixes a few fairly critical errors.

  • makeIdentifiers is now imported by default when rnlp is imported.
  • Fixed a critical error in parse.makeIdentifiers() where the lateWordInSentence predicate was never created. In its place a second (and erroneous) lateSentenceInBlock predicate was created instead.
  • The way punctuation was previously handled did not quite work correctly. text_string.strip(_punctuation) worked in most simple cases, but certain punctuation marks within sentences were problematic for BoostSRL's parser.

hayesall added 5 commits May 31, 2018 10:50
…d was at the end of the sentence, there may have been errors when running in practice.
…stead a second lateSentenceInBlock predicate was created). beginning and ending variables were being overwritten in the inner loop, so lateSentenceInBlock was almost never created.
…here it runs parse.makeIdentifiers, but this case only runs the code to ensure errors are not thrown, it does not validate that outputs match any expectation.
@codecov-io
Copy link

codecov-io commented May 31, 2018

Codecov Report

Merging #7 into master will increase coverage by 43.12%.
The diff coverage is 94.2%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master       #7       +/-   ##
===========================================
+ Coverage   55.14%   98.26%   +43.12%     
===========================================
  Files           5        7        +2     
  Lines         214      231       +17     
===========================================
+ Hits          118      227      +109     
+ Misses         96        4       -92
Impacted Files Coverage Δ
rnlp/tests/rnlptests/test_textprocessing.py 100% <100%> (ø) ⬆️
rnlp/textprocessing.py 100% <100%> (ø) ⬆️
rnlp/tests/rnlptests/test_parse.py 100% <100%> (ø)
rnlp/tests/rnlptests/test_converter.py 100% <100%> (ø)
rnlp/__init__.py 100% <100%> (+18.75%) ⬆️
rnlp/parse.py 96.19% <90.24%> (+79.97%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78b2149...f7a2088. Read the comment docs.

@hayesall hayesall merged commit f7a2088 into master Jun 1, 2018
@hayesall hayesall deleted the parse branch June 1, 2018 14:35
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.

2 participants