Use JS regex syntax #609
Use JS regex syntax #609
Conversation
Codecov Report
@@ Coverage Diff @@
## master #609 +/- ##
==========================================
+ Coverage 96.1% 96.11% +<.01%
==========================================
Files 18 18
Lines 2416 2417 +1
Branches 306 306
==========================================
+ Hits 2322 2323 +1
Misses 79 79
Partials 15 15 |
| @@ -92,10 +93,10 @@ def find_additional_properties(instance, schema): | |||
| """ | |||
|
|
|||
| properties = schema.get("properties", {}) | |||
| patterns = "|".join(schema.get("patternProperties", {})) | |||
| patterns = "|".join(sorted(schema.get("patternProperties", {}))) | |||
There was a problem hiding this comment.
Sorting ensures that this is always a single entry in the regex cache, which is quite important for performance (of this tiny function, anyway).
|
Hooray, thanks! Can you also enable the tests for this in the test suite? Should basically just be adding a line like this: https://github.com/Julian/jsonschema/blob/master/jsonschema/tests/test_jsonschema_test_suite.py#L71 to each of the drafts. |
|
Done! I've left out Draft3 because it doesn't have the relevant test cases upstream, which I think is json-schema-org/JSON-Schema-Test-Suite#274. But drafts 4, 6, and 7 should all be validated... and there's a couple of failures. Welp. ...I think I'll need to take a little longer to debug that, and ensure that I got the everything right upstream 😞 |
Note that js_regex.compile has a cache, just like re.compile, so calling it in loops does a lot of dict lookups but very little work.
|
@Julian - I think this just need to be run on top of the fixed version of my upstream tests, and we'll be good to go 😄 |
433ab2f0 Merge pull request python-jsonschema#286 from Zac-HD/not-patterns dbaa3aac Fix data - escape unicode, not regex pattern git-subtree-dir: json git-subtree-split: 433ab2f0fd6b981527e838cd149c91d823bdc367
* commit 'cdee16959760fe0cd599074cb5d7199eb6ef0d54': Squashed 'json/' changes from 2d554504..433ab2f0
|
Think we're getting there! I think there's still one that looks like it's failing: https://travis-ci.org/Julian/jsonschema/jobs/594558002#L3834 |
|
Aaaah, I see what's happening there: this isn't one of the "does it match" tests I added - it's actually for |
|
And done! ✨🎉🚀 |
|
Awesome, well done man, really appreciated. |
|
@Julian - any advice on when this will make it to PyPI? I've been asked to support it in If it's going to be a while I can write a shim, but I'd feel pretty silly if I did that and you updated the day after! |
|
Probably today :)
…On Wed, Oct 9, 2019, 06:49 Zac Hatfield-Dodds ***@***.***> wrote:
@Julian <https://github.com/Julian> - any advice on when this will make
it to PyPI? I've been asked to support it in hypothesis-jsonschema, but
doing so before jsonschema itself will cause lots of test failures...
If it's going to be a while I can write a shim, but I'd feel pretty silly
if I did that and you updated the day after!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#609>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACQQXT4B22B3RQ5C4VTFSTQNWZKTANCNFSM4I5KFSOQ>
.
|
|
Awesome! I'll aim to do my integration tomorrow then 😁 |
|
(Done) :)
…On Wed, Oct 9, 2019 at 8:44 AM Zac Hatfield-Dodds ***@***.***> wrote:
Awesome! I'll aim to do my integration tomorrow then 😁
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#609>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACQQXT5Z7S2GZI76FZXEYDQNXGZLANCNFSM4I5KFSOQ>
.
|
Note that
js_regex.compilehas a cache, just likere.compile, so calling it in loops does a lot of dict lookups but very little work.Closes #607.