Add support for ASDF serialisation and deserialisation #776

Cadair · 2024-11-06T09:57:03Z

This PR adds support for saving and loading the vast majority of ndcube objects to/from ASDF files.

Thanks to @ViciousEagle03 who did the majority of this work.

Add support for Add serialization logic for uncertainty objects astropy/asdf-astropy#239
Add support for Fits wcs astropy/asdf-astropy#246
Add support for MultipleTableCoordinate
Add documentation
Bump all schemas & tags to 1.0
Add support for NDMeta
Test the few untested lines in converters
Validate manually that having a reference to the parent cube in extra and global coords doesn’t cause trouble.

[asdf] Add some documentation

DanRyanIrish

Initial partial review

docs/explaining_ndcube/asdf_serialization.rst

ndcube/asdf/converters/ndcollection_converter.py

DanRyanIrish · 2025-05-26T12:08:50Z

ndcube/asdf/converters/ndcollection_converter.py

+        key_value_pairs = list(zip(node["keys"], node["value"]))
+        aligned_axes = list(node.get("aligned_axes").values())
+        aligned_axes = tuple(tuple(lst) for lst in aligned_axes)
+        return NDCollection(key_value_pairs, meta=node.get("meta"), aligned_axes=aligned_axes)


Does this mean that every future metadata object needs its own schema to be saved and read?

Well yes and no, anything custom will need one.

This also goes for unsaveable values in meta. i.e. if you put a dask array in your meta dict it will break.

DanRyanIrish · 2025-05-26T12:18:40Z

ndcube/asdf/converters/tablecoord_converter.py

+
+        return node
+
+


Got this far.

Co-authored-by: DanRyanIrish <[email protected]>

Cadair · 2025-05-26T14:00:01Z

ndcube/asdf/resources/schemas/ndcube-1.0.0.yaml

+    anyOf:
+      - tag: "tag:stsci.edu:asdf/core/ndarray-1.*"
+      - type: object
+      - type: boolean


@DanRyanIrish Is having a single boolean as a mask something that's actually logical? We have some test fixtures which do this so I added this here.

Cadair · 2025-05-26T14:43:25Z

ndcube/meta.py

@@ -184,11 +184,11 @@ def _sanitize_axis_value(self, axis, value, key):
        if isinstance(axis, numbers.Integral):
            axis = (axis,)
        if len(axis) == 0:
-            return ValueError(axis_err_msg)
+            raise ValueError(axis_err_msg)


This was a fun bug to discover. I ended up with a dict with ValueErrors as the values.

In that case, this probably deserves a bugfix changelog file.

ndcube/asdf/resources/schemas/ndcollection-1.0.0.yaml

DanRyanIrish · 2025-05-27T13:21:32Z

ndcube/asdf/resources/schemas/ndcollection-1.0.0.yaml

+    type: object
+    patternProperties:
+      ".*":
+        oneOf:


Strictly speaking, I think NDCollection can allow any type of item, not just NDCube and NDCubeSequence, more like dict. Items included in aligned_axes must be sliceable and have enough axes for them to include the designated aligned one. But I think other items can be any object...?

Yes it can, but the question is do we want it to? We are more strict in a bunch of these schemas than the code strictly requires.

I guess from a personal perspective keeping it looser here would let me put dkist Dataset objects in, and maybe the same logic for some other subclasses.

Yes it can, but the question is do we want it to?

Saving aside, I don't see why we would restrict it.

We are more strict in a bunch of these schemas than the code strictly requires.

Which would mean that NDCollection can only be saved for the standard use cases, but not all possible?

@braingram you got thoughts on how lax or strict we should be in the schemas? ndcube is a little different from dkist for example where we are often subclassed so I'm hesitant to be too strict about things in these schemas?

DanRyanIrish

Left a couple small comments that may require changes. Didn't look through the tests. Not an ASDF expert, but didn't see anything obvious that I didn't comment on.

braingram

A few comments on the schemas.

ndcube/asdf/resources/schemas/global_coords-1.0.0.yaml

ndcube/asdf/resources/schemas/timetablecoordinate-1.0.0.yaml

braingram · 2025-05-27T13:37:07Z

pyproject.toml

+  "astropy>=5.3.0",
+  "gwcs>=0.20",
+  "numpy>=1.25.0",
+  "scipy>=1.11.1",


Should this have asdf and asdf-astropy?

We should probably add an asdf extra.

Co-authored-by: Brett Graham <[email protected]>

Cadair · 2025-05-27T14:25:25Z

ndcube/asdf/resources/schemas/quantitytablecoordinate-1.0.0.yaml

+  unit:
+    anyOf:
+      - tag: "tag:stsci.edu:asdf/unit/unit-*"
+      - tag: "tag:astropy.org:astropy/units/unit-1.*"


Suggested change

- tag: "tag:astropy.org:astropy/units/unit-1.*"

- tag: "tag:astropy.org:astropy/units/unit-*"

Cadair · 2025-05-27T14:47:03Z

ndcube/asdf/resources/schemas/ndcollection-1.0.0.yaml

+    type: object
+    patternProperties:
+      ".*":
+        oneOf:


Suggested change

oneOf:

anyOf:

Cadair · 2025-05-27T14:47:57Z

ndcube/asdf/resources/schemas/global_coords-1.0.0.yaml

+      items:
+          - type: string
+          - type: object
+            oneOf:


Suggested change

oneOf:

anyOf:

Cadair · 2025-05-27T14:48:15Z

ndcube/asdf/resources/schemas/extra_coords-1.0.0.yaml

+    items:
+        type: array
+        items:
+            - oneOf:


Suggested change

- oneOf:

- anyOf:

Cadair · 2025-05-27T14:48:23Z

ndcube/asdf/resources/schemas/extra_coords-1.0.0.yaml

+            - oneOf:
+              - type: number
+              - type: array
+            - oneOf:


Suggested change

- oneOf:

- anyOf:

Cadair · 2025-05-27T14:55:36Z

ndcube/asdf/resources/schemas/ndcollection-1.0.0.yaml

+      ".*":
+        oneOf:
+          - tag: "tag:sunpy.org:ndcube/ndcube/ndcube-*"
+          - tag: "tag:sunpy.org:ndcube/ndcube/ndcube_sequence-*"


Suggested change

- tag: "tag:sunpy.org:ndcube/ndcube/ndcube_sequence-*"

- tag: "tag:sunpy.org:ndcube/ndcube/ndcube_sequence-*"

# Allow any other objects here as we don't want to restrict what people can save

- {}

ViciousEagle03 and others added 30 commits November 6, 2024 09:25

initial commit

ac36722

version alignment

6be61e7

Remove method to remove any delay and

6e619fc

Update tag and

f505703

enable schema test

af2f9c7

revert small change

adfc6b3

Add the converters for ExtraCoords and TimeTableCoordinate class

78eeb53

Add ExtraCoords and TimeTableCoord Schema

0804f1d

Update manifest and entry_point.py

0f4a86f

Add the QuantityTableCoordinate and SkyCoordTableCoordinate converters

86bfac9

Add the schema for QuantityTableCoordinate and SkyCoordTableCoordinate

da1c14e

Update manifest and entry_point.py

dc24d40

Add validation for schema and manifests

ef208ed

pre-commit

c3d2606

Update the tox.ini and CI workflow

86c2b6c

Update schemas

3a875ab

Add the converter and schema for GlobalCoords

ce5a14b

Update converters

9247b46

Update entry_points,schemas and manifest

c8a1bc4

minor change

c681cb2

lowercase schema URIs and use wildcards

42c590e

Add tests and GWCS objects

7f3a81b

apply suggestions from code review

7622950

Remove mesh as a property validator

f89e195

Update the dependencies version

8b519ae

Style changes and add warnings to NDCube converter

37bf563

env name update

693e63e

Add asdf as an optional dep

de6944e

Remove asdf as an optional dependency

22a839e

Add support for meta in converter and schema

ec863ad

Merge pull request #843 from SolarDrew/asdf-support

45f6d55

[asdf] Add some documentation

Cadair force-pushed the asdf-support branch from 2a27b45 to c6016b9 Compare May 26, 2025 11:43

Bump versions of all schemas etc

350d7ce

Cadair force-pushed the asdf-support branch from c6016b9 to 350d7ce Compare May 26, 2025 11:44

Add changelog

f1cec49

Cadair marked this pull request as ready for review May 26, 2025 11:45

DanRyanIrish reviewed May 26, 2025

View reviewed changes

Cadair and others added 5 commits May 26, 2025 14:00

Bump various deps based on SPEC-0

f430969

American I guess

38e2680

Update 776.feature.rst

da2b74f

Update docs/explaining_ndcube/asdf_serialization.rst

3754a61

Co-authored-by: DanRyanIrish <[email protected]>

More test fixes

5fac69a

Cadair commented May 26, 2025

View reviewed changes

Allow bool for mask

81677e9

Cadair force-pushed the asdf-support branch from 31fb5f3 to c144f77 Compare May 26, 2025 14:40

Add support for NDMeta

b90a2ff

Cadair force-pushed the asdf-support branch from c144f77 to b90a2ff Compare May 26, 2025 14:41

Cadair commented May 26, 2025

View reviewed changes

Allow FITS WCS in ExtraCoords ASDF schema

ad258e5

Cadair commented May 26, 2025

View reviewed changes

ndcube/asdf/resources/schemas/ndcollection-1.0.0.yaml Outdated Show resolved Hide resolved

Change NDCollection to save a dict not kv pairs

e2c24f2

Cadair commented May 27, 2025

View reviewed changes

ndcube/asdf/resources/schemas/ndcollection-1.0.0.yaml Show resolved Hide resolved

DanRyanIrish reviewed May 27, 2025

View reviewed changes

braingram reviewed May 27, 2025

View reviewed changes

Apply suggestions from code review

85f93f7

Co-authored-by: Brett Graham <[email protected]>

Cadair commented May 27, 2025

View reviewed changes

	- tag: "tag:astropy.org:astropy/units/unit-1.*"
	- tag: "tag:astropy.org:astropy/units/unit-*"

Uh oh!

Add support for ASDF serialisation and deserialisation #776

Are you sure you want to change the base?

Add support for ASDF serialisation and deserialisation #776

Conversation

Cadair commented Nov 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DanRyanIrish left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DanRyanIrish May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanRyanIrish left a comment

Choose a reason for hiding this comment

Uh oh!

braingram left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Cadair May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Cadair commented Nov 6, 2024 •

edited

Loading

DanRyanIrish May 27, 2025 •

edited

Loading

Cadair May 27, 2025 •

edited

Loading