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

Schema updates for current and upcoming asdf changes #910

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

braingram
Copy link
Contributor

@braingram braingram commented Feb 2, 2024

Changes

asdf is working on making the current development version of the ASDF standard (1.6.0) stable (see asdf-format/asdf#1744 for details). As the 1.6.0 version contains new versions of a few core schemas (including ndarray). This PR updates the weldx schemas for these upcoming changes (and applies a few other related fixes).

  • update schema examples that use ndarray-1.0.0 adding a asdf-standard requirement to force these examples to use ASDF standard 1.5.0 (where ndarray-1.0.0 exists)
  • update schema "id"s to match "uri"s used to register the schemas with asdf
  • update manifest generation script to use the uri prefix used to register the manifests (see below)

For the manifests, the id in the files follow the form .../weldx/manifests/...

id: asdf://weldx.bam.de/weldx/manifests/weldx-0.1.2

However these files are registered:
mapping = DirectoryResourceMapping(
MANIFEST_PATH,
WELDX_EXTENSION_URI_BASE,
recursive=True,
filename_pattern="*.yaml",
stem_filename=True,
)

using the WELDX_EXTENSION_URI_BASE uri prefix:
WELDX_EXTENSION_URI_BASE = "asdf://weldx.bam.de/weldx/extensions/"

which uses .../weldx/extensions. This means that these manifest resources are available (via the asdf resource_manager) using ../weldx/extensions/... uris yet have ids with .../weldx/manifests/

>> import asdf
>> manifest = asdf.schema.load_schema("asdf://weldx.bam.de/weldx/extensions/weldx-0.1.2")
>> manifest['id']
asdf://weldx.bam.de/weldx/manifests/weldx-0.1.2

This PR changes the ids to match the uris.

This PR also updates SerializationContext imports to use asdf.extension for asdf version >= 3.0.0 to avoid DeprecationWarning messages form importing asdf.asdf and SerializationContext from asdf.asdf.

Related Issues

Fixes #900

Checks

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

Copy link

github-actions bot commented Feb 2, 2024

Test Results

2 188 tests  ±0   2 187 ✅ ±0   2m 2s ⏱️ +3s
    1 suites ±0       1 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 9daf287. ± Comparison against base commit c8457bf.

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Mar 12, 2024

Codecov Report

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

Project coverage is 96.12%. Comparing base (5ad2929) to head (9daf287).
Report is 30 commits behind head on master.

Files with missing lines Patch % Lines
weldx/asdf/file.py 50.00% 1 Missing ⚠️
weldx/asdf/types.py 75.00% 1 Missing ⚠️
weldx/asdf/util.py 75.00% 1 Missing ⚠️
weldx/tags/base_types.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #910      +/-   ##
==========================================
- Coverage   96.48%   96.12%   -0.37%     
==========================================
  Files          95       95              
  Lines        6293     6347      +54     
==========================================
+ Hits         6072     6101      +29     
- Misses        221      246      +25     

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


🚨 Try these New Features:

@braingram braingram force-pushed the schema_cleanup branch 2 times, most recently from 104dcdf to f9dca49 Compare March 22, 2024 15:17
@braingram braingram marked this pull request as ready for review March 22, 2024 15:20
@braingram braingram changed the title updates schema ids to match uris Schema updates for current and upcoming asdf changes Mar 22, 2024
@CagtayFabry
Copy link
Member

Hi @braingram,
thank you for the explanations.

I am planning a rework on some of the code for asdf>=3 (hopefully soon)
In that light would it make sense to include this as a transition and remove any asdf 2.x related code in the next major release of weldx?

@braingram
Copy link
Contributor Author

Thanks for taking a look!

I am planning a rework on some of the code for asdf>=3 (hopefully soon) In that light would it make sense to include this as a transition and remove any asdf 2.x related code in the next major release of weldx?

That sounds great. Let me know if there's anything I can do to help (and feel free to ping me when the changes are ready for review or if you run into any issues). There are a few changes in 3.1.0 (changelog here) that might be useful including:

@CagtayFabry
Copy link
Member

Thank you, I followed the 3.0/3.1 release notes and the changes will make for some big QOL improvements here!

@braingram
Copy link
Contributor Author

FYI the asdf development branch will soon be transitioning to asdf 4.0 where ASDF standard 1.6.0 will be the default.

The next version of the ASDF standard (1.6.0) contains a new
version of the ndarray schema (ndarray-1.1.0). Examples that
explicitly use ndarray-1.0.0 will only work with the older
standard versions (<1.6.0). This doesn't impact file
reading and writing (as if 1.5.0 is used for write, ndarray-1.0.0
will be used, if 1.6.0 is used, ndarray-1.1.0 will be used). However
the schema examples require hard-coded yaml that is less flexible
than real files and require a restriction like that added in
this commit.
@braingram
Copy link
Contributor Author

Following up on #952 I modified this PR to also unpin asdf.

I also noticed that there is an import of AsdfSpec that's used in format_tag:

weldx/weldx/asdf/types.py

Lines 120 to 121 in c8457bf

if isinstance(version, AsdfSpec):
version = str(version.spec)

AsdfSpec was deprecated in asdf 3.3.0 and I can't find any places in weldx where an AsdfSpec is provided to format_tag so I updated this PR to remove the AsdfSpec usage. See asdf-format/asdf#1772 for motivation why AsdfSpec was removed.

@braingram
Copy link
Contributor Author

Also tests like this one:

with pytest.raises(ValidationError) as e:
WeldxFile(tree=dict(foo="bar"), custom_schema=schema, mode="rw")

appear to be relying on asdf validating a tree provided via AsdfFile(tree). This behavior was deprecated in asdf 3.1.0 (see asdf-format/asdf#1691) and removed in asdf 4.0.0.

I modified this PR to call AsdfFile.validate during WeldxFile._write_tree for asdf >= 4.0.0. (asdf 4.0.0 validates on write so the modification here might be a bit confusing. I picked this function since it's what's called during WeldxFile.init).

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.

ASDF deprecation warnings
2 participants