-
Notifications
You must be signed in to change notification settings - Fork 28
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
Update tests for asdf 3.0 #1004
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left a few inline comments about the changes.
0 warnings""" | ||
else: | ||
expected_out = f"""Certifying '{jwst_data}/invalid.asdf' (1/1) as 'ASDF' relative to context 'jwst.pmap' | ||
instrument='UNKNOWN' type='UNKNOWN' data='{jwst_data}/invalid.asdf' :: Validation error : Does not appear to be a ASDF file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This warning message was changed as asdf 3.0 no longer support FITS files: https://asdf.readthedocs.io/en/3.0.0/asdf/whats_new.html#removed-features
Setting 'META.EXPOSURE.TYPE [EXP_TYPE]' = None to value of 'META.EXPOSURE.P_EXPTYPE [P_EXPTYP]' = 'FGS_IMAGE|FGS_FOCUS|FGS_INTFLAT|FGS_SKYFLAT|' | ||
File written with dev version of asdf library: 2.0.0.dev1213 | ||
Checking JWST datamodels. | ||
######################################## | ||
0 errors | ||
1 warnings""" | ||
else: | ||
expected_out = f""" Certifying '{jwst_data}/jwst_fgs_distortion_bad_asdf_version.asdf' (1/1) as 'ASDF' relative to context 'jwst_0591.pmap' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is invalid and no longer (in asdf 3.0) loads as a custom object (and instead is returned as a simple tagged dictionary). The main issue is that the file:
#ASDF 1.0.0
#ASDF_STANDARD 1.2.0
%YAML 1.1
%TAG ! tag:stsci.edu:asdf/
--- !core/asdf-1.0.0
claims to be asdf-standard 1.2.0 which only supports core/asdf-1.1.0
not core/asdf-1.0.0
:
https://asdf-standard.readthedocs.io/en/1.0.3/generated/asdf-format.org/core/core-1.2.0.html
The expected output was updated to capture this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file contains an invalid block index. This does not prevent the file from loading (in asdf 3.0 or earlier versions). However, in asdf 3.0 a warning is issued (to indicate that the file has this issue):
AsdfBlockIndexWarning: Invalid block index contents for block 0, falling back to serial reading: Invalid block magic
The addition of this new warning conflicts with the strict output checking done within the crds tests. As the block index is optional, the block index was removed from this file so it will load in asdf 3.0 and older versions with no warnings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file has an invalid block index and received the same fix as jwst_nircam_specwcs_1_4_0.asdf
.
@@ -177,11 +179,25 @@ def test_diff_asdf(capsys, jwst_shared_cache_state, jwst_data): | |||
|
|||
output = capsys.readouterr().out | |||
|
|||
assert output == """ ndarrays differ by contents | |||
ndarrays differ by contents |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The output of asdftool diff
changed in asdf 3 and no longer matches this expected output. asdf-format/asdf#1672 fixes the issue that caused this test to raise an exception and improved the output to better match the other tree differences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks Brett
This PR addresses a number of crds test and asdf 3.0 incompatibilities. See comments below for details.