BAMWelDX / weldx

The welding data exchange format
https://www.bam.de/weldx
BSD 3-Clause "New" or "Revised" License
19 stars 9 forks source link

import ValidationError from asdf.exceptions #886

Closed braingram closed 11 months ago

braingram commented 11 months ago

Changes

asdf has recently run into compatibility issues with jsonschema 4.18+. The newer versions of jsonschema dropped support for a feature required in asdf which left us with little choice but to vendorize jsonschema. This presented an opportunity to clean up some leaky bits of the asdf API including the issue that jsonschema.ValidationError was transparently passed to user code and libraries (like weldx) had to import ValidationError from jsonschema.

asdf 2.15 introduced ValidationError as part of the public api available at asdf.exceptions.ValidationError. The changes in this PR modify a few tests and docs to import ValidationError from asdf.exceptions (instead of jsonschema or from the top level asdf module).

asdf 2.15.1 includes the vendorized jsonschema, keeps jsonschema as a dependency and attempts to use the exceptions from the installed jsonschema (to not break some downstream libraries that use asdf). We are planning to drop jsonschema as a dependency and stop using it's exceptions in asdf 3.0 (or possibly in 2.15.2 if further changes to jsonschema make it's exceptions incompatible).

Please see the What's New page in the asdf 2.15.1 docs for more information and let me know if you have any questions, comments or concerns.

Checks

review-notebook-app[bot] commented 11 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

github-actions[bot] commented 11 months ago

Test Results

2 188 tests  ±0   2 187 :heavy_check_mark: ±0   2m 57s :stopwatch: +18s        1 suites ±0          1 :zzz: ±0         1 files   ±0          0 :x: ±0 

Results for commit cb0a614f. ± Comparison against base commit 80e77576.

:recycle: This comment has been updated with latest results.

codecov[bot] commented 11 months ago

Codecov Report

Merging #886 (cb0a614) into master (80e7757) will decrease coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #886      +/-   ##
==========================================
- Coverage   96.47%   96.47%   -0.01%     
==========================================
  Files          95       95              
  Lines        6293     6292       -1     
==========================================
- Hits         6071     6070       -1     
  Misses        222      222              
Files Changed Coverage Δ
weldx/asdf/validators.py 98.24% <ø> (ø)
weldx/asdf/file.py 96.30% <100.00%> (-0.02%) :arrow_down:
CagtayFabry commented 11 months ago

thanks a lot @braingram for carrying this over !

Does this require asdf=2.15.1 for the new imports?

braingram commented 11 months ago

Thanks!

Either 2.15.0 or 2.15.1 will work (2.15.0 has ValidationError at asdf.exceptions but a bug in the docs prevented it from being listed).

2.15.1 is probably safer as it includes the jsonschema vendorization which means 2.15.0 might break at some point if jsonschema decides to change their excepetion classes in a way that is incompatible with the vendorized version (4.17.3).