Explorer YAML skeleton export (+ fix variable-attribute to_dict_skeleton)#94
Merged
Conversation
VariableAttribute is not a subclass of Attribute, so to_dict_skeleton's
singledispatch found no handler and fell through to the no-op base,
serializing every variable attribute to null. Register a dedicated
VariableAttribute handler that emits the same {values, types} shape as
the global Attribute handler (single-entry lists, since variable
attributes hold one entry) and stringifies time-typed values.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add cdf-yaml.js: a pure, Node-testable YAML emitter plus a buildSkeleton that mirrors pycdfpp.to_dict_skeleton (metadata only — types, shapes, attributes; never array data, so it is safe for high-rank/huge variables). Type codes resolve to names via a static CDF_TYPE_NAMES map, so the serializer needs no WASM. Browser-only skeletonFromCdfFile/ cdfFileToYaml adapt the live CdfFile getters. Wire an "Export YAML" button into the Explorer that downloads <name>.skeleton.yaml for the loaded CDF. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #94 +/- ##
==========================================
- Coverage 91.85% 91.11% -0.74%
==========================================
Files 58 58
Lines 3524 3524
==========================================
- Hits 3237 3211 -26
- Misses 287 313 +26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…n export Two correctness fixes from the PR #94 review: - Emitter escaping: mapping keys were interpolated verbatim and needsQuote missed leading YAML indicators (`- `, `? `, `~`) and YAML-1.1 number forms (`1_000`, `0x1a`). Such keys/values re-parsed to a different value or failed to parse, so the skeleton did not round-trip. Quote keys via needsQuote and broaden it (verified by re-parsing the emitted YAML with PyYAML). - Export no longer force-loads variable data. skeletonFromCdfFile re-walked the CDF via get_variable(), which unconditionally calls var.load_values() + copy_values (wacdfpp.cpp), materializing every variable's array — exactly the OOM the high-rank gating guards against, and contradicting the file's own "metadata only" claim. Build the skeleton from the currentModel app.js already holds instead (rawFromCdfFile now also carries per-variable compression + attribute type codes), so export does zero WASM calls and reads no array data. Removes the duplicate CdfFile adapter the review also flagged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- cdf-yaml.js: use `\d` instead of `[0-9]` (S6353). - wacdfpp_yaml test: `Number.NaN` instead of bare `NaN` (S7773). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jeandet
added a commit
that referenced
this pull request
Jun 25, 2026
…n export Two correctness fixes from the PR #94 review: - Emitter escaping: mapping keys were interpolated verbatim and needsQuote missed leading YAML indicators (`- `, `? `, `~`) and YAML-1.1 number forms (`1_000`, `0x1a`). Such keys/values re-parsed to a different value or failed to parse, so the skeleton did not round-trip. Quote keys via needsQuote and broaden it (verified by re-parsing the emitted YAML with PyYAML). - Export no longer force-loads variable data. skeletonFromCdfFile re-walked the CDF via get_variable(), which unconditionally calls var.load_values() + copy_values (wacdfpp.cpp), materializing every variable's array — exactly the OOM the high-rank gating guards against, and contradicting the file's own "metadata only" claim. Build the skeleton from the currentModel app.js already holds instead (rawFromCdfFile now also carries per-variable compression + attribute type codes), so export does zero WASM calls and reads no array data. Removes the duplicate CdfFile adapter the review also flagged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two related changes around CDF skeleton serialization:
1. Fix:
pycdfpp.to_dict_skeletondrops variable attributesVariableAttributeis not a subclass ofAttribute, so thesingledispatch-basedto_dict_skeletonfound no matching handler and fell through to the no-op base — serializing every variable attribute tonull.Registers a dedicated
VariableAttributehandler emitting the same{values, types}shape as the globalAttributehandler (single-entry lists, since variable attributes hold exactly one entry) and stringifies time-typed values.2. Feature: YAML skeleton export in the CDFpp Explorer
New pure, Node-testable
wacdfpp/cdf-yaml.js:toYaml(value)— minimal YAML emitter for the restricted skeleton value space (nested maps, scalar/nested arrays incl. typed arrays, strings quoted only when risky,.nan/.inffor fill values).buildSkeleton(rawVars, rawGlobals, fileMeta)— produces the same structure asto_dict_skeleton; resolves CDF type codes to names via a staticCDF_TYPE_NAMESmap (no WASM needed → fully unit-testable).skeletonFromCdfFile/cdfFileToYaml— browser-only adapters over the liveCdfFilegetters.An "Export YAML" button in the Explorer downloads
<name>.skeleton.yamlfor the loaded CDF. It is metadata-only (types/shapes/attributes, never array data), so it is inherently safe for high-rank / huge variables — consistent with the existing plot/table gating.Test plan
tests/python_skeletons/test.py— newtest_variable_attributes_are_serialized(fails before the fix, passes after).tests/wacdfpp_yaml/test.mjs— 41 checks pinning the emitter format andbuildSkeletonshape (registered intests/meson.build).meson test→ all green, no regressions..nanpreserved, nested arrays intact).Notes
wacdfpp/meson.build) gainscdf-yaml.jsinextra_files+ acopyfile, mirroring the existing JS modules. Rebuild with emsdk to pick it up into the served app; the emitter logic itself is verified against the already-built module.🤖 Generated with Claude Code