[Feature] Support for JSON `minimum` , `maximum`, `exclusiveMinimum`, and `exclusiveMaximum` #1023

hudson-ai · 2024-09-13T22:23:55Z

JSON support for integer and float bounds.

Closes #1019 (thanks for the code @mmoskal!)

Co-authored-by: Michał Moskal <[email protected]>

codecov-commenter · 2024-09-16T16:59:13Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 98.06763% with 4 lines in your changes missing coverage. Please review.

Project coverage is 63.04%. Comparing base (6eb08f4) to head (ac64849).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
guidance/library/_regex_utils.py	97.54%	4 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

❗ There is a different number of reports uploaded between BASE (6eb08f4) and HEAD (ac64849). Click for more details.

HEAD has 56 uploads less than BASE

Flag BASE (6eb08f4) HEAD (ac64849)

124 68

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1023      +/-   ##
==========================================
- Coverage   70.25%   63.04%   -7.22%     
==========================================
  Files          62       63       +1     
  Lines        4472     4670     +198     
==========================================
- Hits         3142     2944     -198     
- Misses       1330     1726     +396

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hudson-ai · 2024-09-19T21:42:26Z

Note: we have one xfail here due to the fact that JSON apparently allows trailing .00000... on integers. We could add support for this in the future, but I don't really see a reason to (as doing so doesn't really add any expressive power to a model). Keeping the xfail around as a little piece of "documentation".

Ready for review.

hudson-ai · 2024-09-19T21:45:28Z

Another note: JSON Schema Draft4 has a slightly different convention around exclusiveMinimum/exclusiveMaximum (where they are treated as bools). We may want to allow schemas using this convention, just because I still see a lot of Draft4 in the wild.

Currently, our Draft202012 validator will reject these schemas, so we should be safe not to handle that case internally. Allowing Draft4 might open a bit of a can of worms, so I want to punt on that for at least a while.

hudson-ai · 2024-09-27T18:55:21Z

@riedgar-ms let me know if you're satisfied with the tests here :)

riedgar-ms

My main concern is the lack of negative number tests in TestBoundedNumeric.

riedgar-ms · 2024-09-28T02:07:39Z

tests/unit/library/test_json.py

+                *(5.0, {"type": "integer", "minimum": 5}, True),
+                marks=pytest.mark.xfail(reason="JSON technically allows trailing zeroes, but we currently don't")
+            ),
+            (5.1, {"type": "integer", "minimum": 5}, False),


"Integer" and "5.1" .... is this telling me something about JSON schema that I don't want to know?

Don't worry, this is a "should fail" case (see the False in the third slot of the tuple 😄)

Probably should have made the comment on the previous line :-) The 'reason' gives me bad feelings about potentially subtle bugs (but we have to follow the JSON standard, so 🤷)

riedgar-ms · 2024-09-28T02:09:46Z

tests/unit/library/test_json.py

+            (9.9, {"type": "number", "exclusiveMinimum": 5.0, "exclusiveMaximum": 10.0}, True),
+            # --- Edge cases ---
+            (0, {"type": "integer", "minimum": 0}, True),
+            (-1, {"type": "integer", "minimum": 0}, False),


Is there a reason that these are the only tests of negative numbers?

No, that was a terrible oversight on my part. There are a lot more test cases on the underlying regex_utils, but I'll make sure to get some populated in the higher-level json tests.

Just the core cases should be enough. Don't need to re-test everything exhaustively.

riedgar-ms · 2024-09-28T02:13:54Z

tests/unit/library/test_json.py

+            (6, {"type": "integer", "maximum": 5.5}, False),
+            (5, {"type": "integer", "exclusiveMaximum": 5.5}, True),
+            (6, {"type": "integer", "exclusiveMaximum": 5.5}, False),
+            # --- Large numbers ---


We could add a bunch more large number and precision cases (as I think manically about how floating point can really Mess You Up), but I wouldn't worry right now.

riedgar-ms · 2024-09-28T02:16:07Z

tests/unit/library/test_regex_utils.py

+        if not right_inclusive and right_int == right:
+            right_int -= 1
+        do_test_int_range(rx, left_int, right_int, left_none, right_none)
+    eps = 0.000001


This looks like a float epsilon, not a double one?

This was just supposed to be a small number. I think we could probably test with 0.0000000001 all way down to 0.1 each 10x or something. AFAIK JSON schema doesn't say numbers are doubles.

I wouldn't worry about that right now; my main concern is having more negative numbers in the main test cases. I have the impression that JSON may be a bit relaxed in its concepts of floating point.

hudson-ai · 2024-10-01T22:49:20Z

@riedgar-ms added some negative test cases. Will merge once I see some green :)

hudson-ai and others added 13 commits September 13, 2024 14:11

Utilities for generating regexes for ranges of ints and floats

aa068a7

Co-authored-by: Michał Moskal <[email protected]>

Support for unbounded integer ranges

7e09080

Handle json (exclusive) min/max

64a0855

fix (I think) for bounds

2c77ff1

redundant

fd591a5

trust the type checker

b9fa526

else

734e2a0

tests

26a031a

type safety

df0c078

handle min and exclusivemin

1fd725a

draft4

cb53285

remove support for Draft4 exclusiveMin/Max

183688c

remove support for Draft4 validation in tests

6587382

mmoskal and others added 16 commits September 16, 2024 10:05

add exclusive float ranges

903bca3

Merge branch 'main' into json_numeric_ranges

32716df

implement exclusive min/max for json number

dfe4dd9

allow negative zero

d8bcb84

consolidate integer ranges

c918a17

consolidate float ranges

562c318

revert do_test_int_range back to simpler times

1e1eb4c

first attempt at unbounded float ranges

ee8255b

fix do_int_range

405e611

fix recursion

7f64953

add float half intervals

8c014c7

test both sides unbounded

d6250f5

test integer singletons

9703ac8

test float singletons

3900ae6

enable half open json ranges

80bf52f

redo json numeric range test cases

b12ef88

hudson-ai added 2 commits September 19, 2024 14:31

enable trailing zeroes on upper end of json range

0d6e885

xfail integer with trailing .0

566644f

hudson-ai changed the title ~~[WIP] [Feature] Support for JSON minimum , maximum, exclusiveMinimum, and exclusiveMaximum~~ [Feature] Support for JSON minimum , maximum, exclusiveMinimum, and exclusiveMaximum Sep 19, 2024

hudson-ai requested a review from riedgar-ms September 19, 2024 21:42

hudson-ai added 2 commits September 19, 2024 14:47

Merge branch 'main' into json_numeric_ranges

284a673

Merge branch 'main' into json_numeric_ranges

982f182

riedgar-ms approved these changes Sep 28, 2024

View reviewed changes

hudson-ai added 5 commits October 1, 2024 15:25

negative integer tests

23e0ef1

negative number tests

67d2df4

negative edge tests

74c7fb6

large numbers

efedc7e

precision

ac64849

hudson-ai merged commit 63bfd9f into guidance-ai:main Oct 1, 2024
88 of 100 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Support for JSON `minimum` , `maximum`, `exclusiveMinimum`, and `exclusiveMaximum` #1023

[Feature] Support for JSON `minimum` , `maximum`, `exclusiveMinimum`, and `exclusiveMaximum` #1023

hudson-ai commented Sep 13, 2024 •

edited

Loading

codecov-commenter commented Sep 16, 2024 •

edited

Loading

hudson-ai commented Sep 19, 2024

hudson-ai commented Sep 19, 2024

hudson-ai commented Sep 27, 2024

riedgar-ms left a comment

riedgar-ms Sep 28, 2024

hudson-ai Sep 30, 2024

riedgar-ms Sep 30, 2024

riedgar-ms Sep 28, 2024

hudson-ai Sep 30, 2024

riedgar-ms Sep 30, 2024

riedgar-ms Sep 28, 2024

riedgar-ms Sep 28, 2024

mmoskal Sep 28, 2024

riedgar-ms Sep 30, 2024

hudson-ai commented Oct 1, 2024

[Feature] Support for JSON minimum , maximum, exclusiveMinimum, and exclusiveMaximum #1023

[Feature] Support for JSON minimum , maximum, exclusiveMinimum, and exclusiveMaximum #1023

Conversation

hudson-ai commented Sep 13, 2024 • edited Loading

codecov-commenter commented Sep 16, 2024 • edited Loading

Codecov Report

hudson-ai commented Sep 19, 2024

hudson-ai commented Sep 19, 2024

hudson-ai commented Sep 27, 2024

riedgar-ms left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hudson-ai commented Oct 1, 2024

[Feature] Support for JSON `minimum` , `maximum`, `exclusiveMinimum`, and `exclusiveMaximum` #1023

[Feature] Support for JSON `minimum` , `maximum`, `exclusiveMinimum`, and `exclusiveMaximum` #1023

hudson-ai commented Sep 13, 2024 •

edited

Loading

codecov-commenter commented Sep 16, 2024 •

edited

Loading