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

import ValidationError from asdf.exceptions #886

Merged
merged 6 commits into from
Aug 14, 2023

Conversation

braingram
Copy link
Contributor

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

  • updated CHANGELOG.md
  • updated tests/
  • updated doc/
  • update example/tutorial notebooks
  • update manifest file

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions
Copy link

github-actions bot commented Aug 10, 2023

Test Results

2 188 tests  ±0   2 187 ✔️ ±0   2m 57s ⏱️ +18s
       1 suites ±0          1 💤 ±0 
       1 files   ±0          0 ±0 

Results for commit cb0a614. ± Comparison against base commit 80e7757.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Aug 10, 2023

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%) ⬇️

@CagtayFabry
Copy link
Member

thanks a lot @braingram for carrying this over !

Does this require asdf=2.15.1 for the new imports?

@braingram
Copy link
Contributor Author

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).

@CagtayFabry CagtayFabry added ASDF everything ASDF related (python + schemas) dependencies changes in package dependencies labels Aug 14, 2023
@CagtayFabry CagtayFabry self-assigned this Aug 14, 2023
@CagtayFabry CagtayFabry self-requested a review August 14, 2023 08:51
@CagtayFabry CagtayFabry merged commit 7bc16a1 into BAMWelDX:master Aug 14, 2023
23 checks passed
@braingram braingram deleted the asdf_validationerror branch August 14, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASDF everything ASDF related (python + schemas) dependencies changes in package dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants