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

Implement index adjustment for rel json ptr #47

Merged
merged 5 commits into from
Feb 23, 2023

Conversation

handrews
Copy link
Contributor

@handrews handrews commented Feb 18, 2023

Fixes #23 : Adds support for index adjustment to class RelativeJSONPointer per the most recent draft, as modified by recently proposed fixes that correct errors in the original wording for index adjustment.

@marksparkza I've tried to match styles, but I'm perfectly happy to change formatting or wording or whatever so feel free to nitpick. I've not used type hinting in Python before so if I missed something obvious that I should have done with types it's because I don't know what I'm doing :-)

For the tests, I matched the sort of tests that were already present. I noticed some things that were not covered, such as some syntax error conditions, but I have filed those ideas as issue #48.

I started writing a tutorial page for RelativeJSONPointer, but decided to submit it as a separate PR.

With the adjustment to hypothesis 6.0.3 per #45 , all tests run and pass on Python 3.8, 3.9, 3.10, and 3.11.

jsonpointer.py coverage HTML page header before:

Coverage for jschon/jsonpointer.py: 93%
143 statements    134 run    9 missing    11 excluded    4 partial

jsonpointer.py coverage HTML page header after:

Coverage for jschon/jsonpointer.py: 96%
143 statements    137 run    6 missing    11 excluded    3 partial

Test & coverage report before:

------------------------------------------------------------------------------------------------- benchmark: 5 tests -------------------------------------------------------------------------------------------------
Name (time in us)                      Min                    Max                  Mean                StdDev                Median                 IQR            Outliers          OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_create_json                   40.0420 (1.0)      22,765.2910 (49.10)       55.9223 (1.0)        241.7498 (6.51)        41.2089 (1.0)        2.4159 (1.0)        2;2022  17,881.9634 (1.0)        8983           1
test_evaluate_json[valid]         151.0840 (3.77)     15,631.1250 (33.72)      172.3141 (3.08)       256.0344 (6.89)       157.4580 (3.82)       4.5430 (1.88)        1;583   5,803.3546 (0.32)       3702           1
test_evaluate_json[invalid]       180.6669 (4.51)        463.6250 (1.0)        199.4462 (3.57)        37.1391 (1.0)        186.0000 (4.51)       4.7920 (1.98)      558;601   5,013.8835 (0.28)       3268           1
test_create_schema                481.7080 (12.03)    25,930.1249 (55.93)      546.1360 (9.77)       898.4470 (24.19)      496.2500 (12.04)     22.5734 (9.34)         3;43   1,831.0458 (0.10)       1463           1
test_validate_schema            2,487.3330 (62.12)    43,008.5000 (92.77)    3,375.4863 (60.36)    4,836.1567 (130.22)   2,542.6251 (61.70)    635.9163 (263.22)        6;6     296.2536 (0.02)        367           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Legend:
  Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
  OPS: Operations Per Second, computed as 1 / Mean
========================================================================= 2396 passed in 113.10s (0:01:53) ==========================================================================
py310: commands_post[0]> coverage report
Name                              Stmts   Miss Branch BrPart  Cover
-------------------------------------------------------------------
jschon/__init__.py                   20      2      2      0    91%
jschon/catalog/_2019_09.py           18      0      0      0   100%
jschon/catalog/_2020_12.py           18      0      0      0   100%
jschon/catalog/__init__.py          136     11     30      3    89%
jschon/catalog/_next.py              18      0      0      0   100%
jschon/exceptions.py                 14      0      0      0   100%
jschon/formats.py                     7      0      4      1    91%
jschon/json.py                      210     11     86      4    95%
jschon/jsonpatch.py                 169     43     60     15    70%
jschon/jsonpointer.py               128      6     40      2    95%
jschon/jsonschema.py                263     18    121     11    90%
jschon/output.py                     80      2     41      1    96%
jschon/uri.py                        68      2     14      2    95%
jschon/utils.py                      40      5     12      2    87%
jschon/vocabulary/__init__.py        69      6     24      5    86%
jschon/vocabulary/annotation.py      36      0      2      0   100%
jschon/vocabulary/applicator.py     254      0    147      2    99%
jschon/vocabulary/core.py           123     14     36      8    85%
jschon/vocabulary/format.py          28      0      4      0   100%
jschon/vocabulary/legacy.py         100      2     58      5    96%
jschon/vocabulary/validation.py     159      0     68      1    99%
-------------------------------------------------------------------
TOTAL                              1958    122    749     62    92%

Test & coverage report after:

------------------------------------------------------------------------------------------------- benchmark: 5 tests -------------------------------------------------------------------------------------------------
Name (time in us)                      Min                    Max                  Mean                StdDev                Median                 IQR            Outliers          OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_create_json                   40.5411 (1.0)         735.0410 (1.52)        55.1769 (1.0)         36.5315 (1.10)        42.3340 (1.0)        4.1666 (1.0)        61;494  18,123.5242 (1.0)        2164           1
test_evaluate_json[valid]         153.1660 (3.78)        482.3339 (1.0)        170.9793 (3.10)        33.1258 (1.0)        159.5829 (3.77)       4.5832 (1.10)      631;686   5,848.6608 (0.32)       4201           1
test_evaluate_json[invalid]       181.4590 (4.48)     15,583.5000 (32.31)      203.9511 (3.70)       244.8774 (7.39)       187.1660 (4.42)       5.1241 (1.23)       18;743   4,903.1356 (0.27)       4034           1
test_create_schema                482.4999 (11.90)    26,491.4590 (54.92)      545.4101 (9.88)       852.8854 (25.75)      498.5830 (11.78)     23.8648 (5.73)         3;72   1,833.4826 (0.10)       1667           1
test_validate_schema            2,493.6250 (61.51)    46,667.7080 (96.75)    3,446.7382 (62.47)    5,215.5207 (157.45)   2,557.6040 (60.41)    638.9170 (153.34)        6;6     290.1294 (0.02)        360           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Legend:
  Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
  OPS: Operations Per Second, computed as 1 / Mean
========================================================================= 2402 passed in 123.74s (0:02:03) ==========================================================================
py310: commands_post[0]> coverage report
Name                              Stmts   Miss Branch BrPart  Cover
-------------------------------------------------------------------
jschon/__init__.py                   20      2      2      0    91%
jschon/catalog/_2019_09.py           18      0      0      0   100%
jschon/catalog/_2020_12.py           18      0      0      0   100%
jschon/catalog/__init__.py          136     11     30      3    89%
jschon/catalog/_next.py              18      0      0      0   100%
jschon/exceptions.py                 14      0      0      0   100%
jschon/formats.py                     7      0      4      1    91%
jschon/json.py                      210     11     86      4    95%
jschon/jsonpatch.py                 169     43     60     15    70%
jschon/jsonpointer.py               143      6     50      3    95%
jschon/jsonschema.py                263     18    121     11    90%
jschon/output.py                     80      2     41      1    96%
jschon/uri.py                        68      2     14      2    95%
jschon/utils.py                      40      5     12      2    87%
jschon/vocabulary/__init__.py        69      6     24      5    86%
jschon/vocabulary/annotation.py      36      0      2      0   100%
jschon/vocabulary/applicator.py     254      0    147      2    99%
jschon/vocabulary/core.py           123     14     36      8    85%
jschon/vocabulary/format.py          28      0      4      0   100%
jschon/vocabulary/legacy.py         100      2     58      5    96%
jschon/vocabulary/validation.py     159      0     68      1    99%
-------------------------------------------------------------------
TOTAL                              1973    122    759     63    92%

@handrews
Copy link
Contributor Author

It looks like the tests being run on this change are hitting the issue that PR #49 works around.

@marksparkza
Copy link
Owner

@marksparkza I've tried to match styles

up and over. I like it. This kind of style works in jschon 😄 I'm not a huge fan of rigid formatting a la black. Elegance and clarity are paramount.

Adds support for index adjustment to RelativeJSONPointer
per the most recent draft, as modified by the proposed fixes at
handrews/json-schema-spec@main...handrews:json-schema-spec:relptr-more-fixes
This test needed to check against the string '0' rather than
the number 0.

Since the review feedback changed the code to
allow the empty string for the over parameter, this also reworks
the default value testing to handle any combination of parameters
that match their default values, including combinations with
the ref parameter as a JSONPointer instance.
@handrews
Copy link
Contributor Author

handrews commented Feb 20, 2023

@marksparkza I've incorporated your feedback, rebased on top of the workaround for the testing issue, and also fixed a bug I noticed in the test case for creating relative JSON pointers — 0 vs '0' — and reorganized it a bit for more thorough testing of default values and JSONPointer instances. That last part is done as a separate commit in case you want me to split it into another PR or something.

An index adjustment needs an additional parent (the array).
@codecov-commenter
Copy link

codecov-commenter commented Feb 20, 2023

Codecov Report

Base: 90.60% // Head: 90.72% // Increases project coverage by +0.12% 🎉

Coverage data is based on head (996acc1) compared to base (199b8e6).
Patch coverage: 95.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #47      +/-   ##
==========================================
+ Coverage   90.60%   90.72%   +0.12%     
==========================================
  Files          21       21              
  Lines        1958     1973      +15     
  Branches      373      378       +5     
==========================================
+ Hits         1774     1790      +16     
+ Misses        122      121       -1     
  Partials       62       62              
Impacted Files Coverage Δ
jschon/jsonpointer.py 95.10% <95.00%> (+1.35%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Ensure that evaluating a the pointer part of a
Relative JSON Pointer that goes outside of the document
produces an error.
@marksparkza marksparkza merged commit adfd6f6 into marksparkza:main Feb 23, 2023
@handrews handrews deleted the rjp-sideways branch February 27, 2023 02:32
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.

Add support for +/- notation to relative JSON pointers
3 participants