-
-
Notifications
You must be signed in to change notification settings - Fork 416
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
BUGFIX Earley: Now yielding a previously repressed ambiguity #1427
Conversation
P.S. It's easier to see why there should be ambiguity in test_cycle2, if we rename |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chanicpanic Like that? |
Yup, looks good |
@MegaIng Any comments? Or should I just merge it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't say much on the actual implementation changes since I don't know the earley algorithm well enough, but looks good in general.
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1427 +/- ##
==========================================
+ Coverage 89.53% 89.54% +0.01%
==========================================
Files 52 52
Lines 7814 7825 +11
==========================================
+ Hits 6996 7007 +11
Misses 818 818
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Thanks @chanicpanic and @MegaIng ! |
Contains 3 commits:
Test
test_multiple_start_solutions
, that fails before the fixFixes Earley. Previously, slightly different items were considered equal.
A fix to the test_cycle2, which I believe was previously wrong
Issue #1424 was the first lead to finding this bug. But seems that it isn't solved by this change alone.
@chanicpanic Can you please verify my work?