fix dasdae to read multi-dim coordinates #588

d-chambers · 2025-12-06T19:05:11Z

Description

Currently, patches with multi-dimensional coordinates cannot round-trip in the DASDAE format. This PR fixes that.

Checklist

I have (if applicable):

referenced the GitHub issue this PR closes.
documented the new feature with docstrings and/or appropriate doc page.
included tests. See testing guidelines.
added the "ready_for_review" tag once the PR is ready to be reviewed.

Summary by CodeRabbit

Bug Fixes
- Fixed parsing and handling of coordinate dimensions in DAS DAE file I/O. Multi-dimensional coordinates are now correctly preserved during read operations.
Tests
- Added test coverage to verify correct round-trip handling of patches with multi-dimensional coordinates.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-06T19:05:20Z

Walkthrough

Changes to DASDAE I/O coordinate handling: delimiter for parsing coordinate dimension strings changed from "." to ",", and multi-dimensional coordinates are now filtered from kwargs before calling coords.select. Tests added for multi-dimensional coordinate round-tripping.

Changes

Cohort / File(s)	Summary
Coordinate parsing and filtering logic `dascore/io/dasdae/utils.py`	Modified `_get_coords` to use comma delimiter instead of period when parsing coordinate dimension strings; updated `_read_patch` to filter out multi-dimensional coordinates from kwargs before calling `coords.select`.
Multi-dimensional coordinate tests `tests/test_io/test_dasdae/test_dasdae.py`	Added test method `test_roundtrip_coord_multiple_dims` to verify round-tripping of patches with non-dimensional coordinates spanning multiple dimensions (note: test appears duplicated in the same class).

Possibly related PRs

raise error when select on bad patch coord #556: Updates coordinate selection validation and related coordinate handling logic.

Suggested labels

bug, patch

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing DASDAE format to handle multi-dimensional coordinates, which aligns with the code modifications and test additions.
Description check	✅ Passed	The description addresses the core problem (multi-dim coordinates cannot round-trip) and solution, includes the required checklist, and references the issue implicitly through the PR context.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch dasdae_multidim_coords

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2025-12-06T19:08:36Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.92%. Comparing base (1dccf58) to head (10ea91c).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #588   +/-   ##
=======================================
  Coverage   99.92%   99.92%           
=======================================
  Files         129      129           
  Lines       11241    11243    +2     
=======================================
+ Hits        11233    11235    +2     
  Misses          8        8

Flag	Coverage Δ
unittests	`99.92% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

dascore/io/dasdae/utils.py (1)

168-174: Filtering multi-dim coordinates out of read kwargs is reasonable; consider a small guard for the “all dropped” case

This block correctly avoids passing multi-dimensional coordinates into coords.select, which only supports single-dimension selectors, and thus fixes failures when round-tripping patches containing multi-dim non-dimensional coords.

You might consider a small refinement:

If sub_kwargs ends up empty (because all user kwargs referenced multi-dim coords), you could skip calling coords.select and just fall back to the unfiltered data path. That would make the control flow a bit clearer and avoid an unnecessary call that becomes a no-op.

Behavior is otherwise sound and aligned with the goal of enabling multi-dim coord round-trips.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1dccf58 and 10ea91c.

📒 Files selected for processing (2)

dascore/io/dasdae/utils.py (2 hunks)
tests/test_io/test_dasdae/test_dasdae.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

dascore/io/dasdae/utils.py (2)

dascore/core/patch.py (1)

coords (255-269)

dascore/core/coordmanager.py (1)

select (594-636)

tests/test_io/test_dasdae/test_dasdae.py (1)

tests/conftest.py (2)

multi_dim_coords_patch (312-316)

patch (375-377)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)

GitHub Check: test_code (windows-latest, 3.10)
GitHub Check: test_code (windows-latest, 3.13)
GitHub Check: test_code (windows-latest, 3.11)
GitHub Check: test_code (windows-latest, 3.12)
GitHub Check: test_code (macos-latest, 3.12)
GitHub Check: test_code (macos-latest, 3.13)
GitHub Check: test_code (macos-latest, 3.10)
GitHub Check: test_code (ubuntu-latest, 3.12)
GitHub Check: test_code (ubuntu-latest, 3.13)
GitHub Check: test_code (ubuntu-latest, 3.11)
GitHub Check: test_code (ubuntu-latest, 3.10)
GitHub Check: test_code_min_deps (macos-latest, 3.13)
GitHub Check: Run benchmarks
GitHub Check: test_code_min_deps (windows-latest, 3.12)
GitHub Check: test_code_min_deps (ubuntu-latest, 3.12)
GitHub Check: test_code_min_deps (windows-latest, 3.13)
GitHub Check: test_code_min_deps (macos-latest, 3.12)
GitHub Check: test_code_min_deps (ubuntu-latest, 3.13)

🔇 Additional comments (2)

dascore/io/dasdae/utils.py (1)

143-143: Delimiter fix in _get_coords correctly reconstructs coord dimensions

Using value.split(",") matches the write-side behavior (",".join(dims) in _save_coords), so multi-dimensional coordinate dims are now reconstructed as separate dimension names instead of a single combined string. This is a correct and self-contained bugfix.

tests/test_io/test_dasdae/test_dasdae.py (1)

268-285: New multi-dim coord round-trip test is well scoped and effective

This test cleanly captures the regression: a non-dimensional coordinate attached to two dims is written once and then read both from a directory spool and a single file, with patch == patch2 == patch3 ensuring full round-trip equality. It directly guards the behavior introduced in _get_coords and _read_patch without over-specifying implementation details.

codspeed-hq · 2025-12-06T19:36:03Z

CodSpeed Performance Report

Merging #588 will not alter performance

_{Comparing dasdae_multidim_coords (10ea91c) with master (1dccf58)}

Summary

✅ 54 untouched
⏩ 2 skipped¹

2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

fix dasdae to read multi-dim coordinates

10ea91c

coderabbitai bot added bug Something isn't working patch related to Patch class labels Dec 6, 2025

coderabbitai bot reviewed Dec 6, 2025

View reviewed changes

d-chambers merged commit 7ae947a into master Dec 7, 2025
25 checks passed

d-chambers deleted the dasdae_multidim_coords branch December 7, 2025 15:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix dasdae to read multi-dim coordinates #588

fix dasdae to read multi-dim coordinates #588

Uh oh!

d-chambers commented Dec 6, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 6, 2025 •

edited

Loading

Uh oh!

codecov bot commented Dec 6, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

codspeed-hq bot commented Dec 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix dasdae to read multi-dim coordinates #588

fix dasdae to read multi-dim coordinates #588

Uh oh!

Conversation

d-chambers commented Dec 6, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly related PRs

Suggested labels

Pre-merge checks and finishing touches

Uh oh!

codecov bot commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

codspeed-hq bot commented Dec 6, 2025

CodSpeed Performance Report

Merging #588 will not alter performance

Summary

Footnotes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

d-chambers commented Dec 6, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 6, 2025 •

edited

Loading

codecov bot commented Dec 6, 2025 •

edited

Loading