Skip to content

Convert single sample coordinates to dense xdas coordinates - #12

Merged
d-chambers merged 2 commits into
mainfrom
fix/single-sample-coordinate
Aug 29, 2026
Merged

Convert single sample coordinates to dense xdas coordinates#12
d-chambers merged 2 commits into
mainfrom
fix/single-sample-coordinate

Conversation

@d-chambers

@d-chambers d-chambers commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Description

Two independent fixes, both needed to get main green again. Neither changes behaviour for data that already converted successfully.

1. Single sample coordinates

main fails CI on TestDASCorePatch::test_example_patch_to_xdas_dataarray[delta_patch]:

ValueError: `tie_indices` must be strictly increasing

EvenlySampledCoordinate.to_xdas_coord builds an InterpCoordinate from tie_indices, and DASCorePatchConverter._to_base_coords sets those to (0, len(coord) - 1). For an axis holding a single sample that degenerates to (0, 0), which xdas rejects. DASCore's delta_patch example is (1, 200), i.e. one channel, so it trips over this.

This is not specific to that example: any patch with a single channel, or a single time sample, failed to convert to an xdas DataArray.

A single sample axis is now represented as a DenseCoordinate, which has no ordering constraint on its indices. It round trips back to DASCore unchanged, which the existing test_example_patch_round_trip_to_dascore[delta_patch] covers.

Worth noting this is not an xdas regression: xdas has rejected equal tie indices in every release back to 0.2.5. The test suite parametrizes over tuple(EXAMPLE_PATCHES), so it grows as DASCore adds examples, and delta_patch is simply the first one to have a length-one dimension.

2. int32 overflow in the test time coordinates

With the above fixed, the Windows runners then failed two DASPy tests:

FAILED TestDASCorePatch::test_single_distance_patch_to_daspy_section
FAILED TestDASCorePatch::test_array_coordinates_to_daspy_section
ValueError: Array coordinates must be evenly sampled to convert to DASPy.

np.arange defaults to int32 on Windows, and dc.to_timedelta64 scaling those seconds to nanoseconds overflows at nine seconds:

arange(10, dtype=int32): last=2020-01-01T00:00:00.410065408  evenly_sampled=False
arange(10, dtype=int64): last=2020-01-01T00:00:09.000000000  evenly_sampled=True

The error was correct, the time axis really was not evenly sampled. Only the tests use this pattern, so they now ask for int64 explicitly.

Verification

  • Test suite goes from 113 passed / 1 failed to 114 passed locally.
  • The added unit test and the existing delta_patch example test both fail without the source change and pass with it.
  • All CI checks pass, including both Windows jobs, which were the ones failing.
  • Lint is clean under ruff 0.4.8, the version pinned in .pre-commit-config.yaml.

Note

This is independent of #11, which is red for the delta_patch reason above. Merging this first should turn #11 green.

Checklist

I have (if applicable):

  • referenced the GitHub issue this PR closes.
  • documented the new feature with docstrings or appropriate doc page.
  • included a test.
  • added the "ready_for_review" tag once the PR is ready to be reviewed.

An axis holding one sample gives tie indices of (0, 0), which xdas rejects
with "`tie_indices` must be strictly increasing". Converting any patch with
such an axis to an xdas DataArray therefore raised, for example DASCore's
delta_patch example, whose distance axis has a single channel.

Represent a single sample axis as a dense coordinate instead, which has no
such constraint and round trips back to DASCore unchanged.
@codecov-commenter

codecov-commenter commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (3af371f) to head (2ee0f79).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #12   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines          338       340    +2     
=========================================
+ Hits           338       340    +2     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

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

☔ View full report in Codecov by Harness.
📢 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.

np.arange defaults to int32 on Windows. Scaling those seconds to
nanoseconds overflows at nine seconds, so the time axes built from it were
not evenly sampled and the DASPy conversion correctly refused them:

    arange(10, dtype=int32): last=2020-01-01T00:00:00.410065408
    arange(10, dtype=int64): last=2020-01-01T00:00:09.000000000

This only affected the tests, which are the only place the pattern is
used. Ask for int64 explicitly so the axes are the same on every platform.
@d-chambers
d-chambers merged commit 33efbf0 into main Aug 29, 2026
9 checks passed
@d-chambers
d-chambers deleted the fix/single-sample-coordinate branch August 29, 2026 12:03
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.

2 participants