-
Notifications
You must be signed in to change notification settings - Fork 560
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
Fix n3 parser exponent syntax of floats with leading dot. #1012
Conversation
Co-Authored-By: Chris Markiewicz <effigies@gmail.com>
Hopefully I'm not being annoying, but as I was looking at this I wanted to add some tests that should have caught this problem earlier. Made a proposal at white-gecko#1. |
TEST: Stress test exponent regex
@effigies everything fine. We are happy about all good contributions. |
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.
Sorry, assumed Python 2.7 and Python 3 would have done something sensible like use the same method names. Instead we got assertRegex
(3) and assertRegexMatches
(2.7).
These fixes should work across versions.
test/test_n3.py
Outdated
exps = ("1", "12", "+1", "-1", "+12", "-12") | ||
for parts in itertools.product(signs, mantissas, es, exps): | ||
expstring = "".join(parts) | ||
self.assertRegex(expstring, exponent_syntax) |
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.
self.assertRegex(expstring, exponent_syntax) | |
self.assertIsNotNone(exponent_syntax.match(expstring)) |
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 used assetTrue
and assertFalse
which are effectively doing the same.
test/test_n3.py
Outdated
# Add test cases as needed | ||
invalid = (".e1",) | ||
for expstring in invalid: | ||
self.assertNotRegex(expstring, exponent_syntax) |
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.
self.assertNotRegex(expstring, exponent_syntax) | |
self.assertIsNone(exponent_syntax(expstring)) |
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.
Thanks @effigies and @white-gecko for the quick fix! (come on @edmondchuc, you found this, should have fixed it!)
@effigies nice find with the regex. Thanks. |
Fix #999 .