Skip reparsing canonical CoordRange coords in CoordManager update/select

[d-chambers]

What

CoordManager.update / update_coords / select funnel every coordinate
through _get_coord (nested in _get_coord_dim_map), which dumps each coord via
model_dump() and rebuilds it with get_coord(**dict) — re-running full
validation and, for array coords, the monotonic / even-sampling scan — even
when the coordinate was already a valid, canonical object. Because BaseCoord
is a pydantic model, this bypassed get_coord's own
if isinstance(data, BaseCoord): return data fast path.

This adds a single short-circuit for the one type where returning the coord
unchanged is provably a no-op:

if isinstance(coord, CoordRange):
    return coord

A CoordRange is the canonical evenly-sampled representation and round-trips
identically through model_dump() → get_coord().

Why only CoordRange (not all BaseCoords)

The reparse also canonicalizes non-canonical coordinates, so it cannot be
blanket-skipped:

• Array coords (CoordArray / CoordMonotonicArray) can be left
  non-canonical by slicing — e.g. an evenly spaced subset produced by
  __getitem__ during sample selection should collapse to a CoordRange.
• A fully-specified CoordPartial should canonicalize to a CoordRange
  (otherwise value-based selection on it wrongly raises).

Both cases are covered by regression tests. CoordRange was verified (and
independently confirmed in review) to round-trip identically for forward,
reversed, singleton, datetime, unit-bearing, and dtype-bearing variants.

Performance (micro-benchmark on master-equivalent code)

Operation	Before	After
update_coords, CoordRange dims	~393 µs	~22 µs (~17×)
value-based select, CoordRange	~596 µs	~180 µs (~3.3×)

Array-coord paths are intentionally unchanged (they still reparse for
canonicalization). Output is bit-for-bit identical: coordinate arrays, dim maps,
dtypes, data, attrs, and history are unchanged.

Tests / benchmarks

• Identity preservation for CoordRange update/select; value-equivalence for
  array coords; the select result is unchanged.
• Regression guards: even sample-select subset of an irregular coord →
  CoordRange; fully-specified CoordPartial → CoordRange with working
  value-select.
• New test_update_existing_coords CodSpeed benchmark.

Verification

• pytest tests → 6318 passed, 84 skipped, 4 xfailed.
• Doctests for coordmanager.py pass; pre-commit clean.
• Reviewed via Codex CLI across three iterations; two real edge cases it found
  (array-slice and fully-specified-CoordPartial canonicalization) were fixed and
  now have regression tests.

Follow-up to #765 / #757; see .scratch/optimization_ideas.md (PR 3).

[coderabbitai]

Warning

Review limit reached

@d-chambers, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cc26e20e-eabe-48ba-a9ee-706877bc95c3

📥 Commits

Reviewing files that changed from the base of the PR and between 955643d and 6186b7d.

📒 Files selected for processing (3)

• benchmarks/test_patch_benchmarks.py
• dascore/core/coordmanager.py
• tests/test_core/test_coordmanager.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch coord-preserve-basecoord

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[d-chambers]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 6186b7d653

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.93%. Comparing base (af83f76) to head (6186b7d).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #768   +/-   ##
=======================================
  Coverage   99.93%   99.93%           
=======================================
  Files         145      145           
  Lines       12857    12859    +2     
=======================================
+ Hits        12849    12851    +2     
  Misses          8        8           

Flag	Coverage Δ
unittests	99.93% <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 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.

[d-chambers]

Why the guard is CoordRange only (the "canonical coords" reasoning)

The core fact behind this change: the same coordinate values can be stored in more than one representation, and DASCore has a preferred ("canonical") one.

A coordinate is a BaseCoord, which can be any of:

Type	Stores	Used when
CoordRange	start, stop, step	values are evenly spaced
CoordMonotonicArray	the full array	increasing but uneven (e.g. [0, 1, 3, 9])
CoordArray	the full array	not even sorted
CoordPartial	just a length	a dim with a size but no values

[0, 10, 20, 30] is evenly spaced, so its canonical form is CoordRange(start=0, stop=40, step=10) — compact, and it reports evenly_sampled == True. The same values as a CoordMonotonicArray would be correct-but-non-canonical (bulkier, and reports evenly_sampled == False).

get_coord(data=...) is what performs this inference ("evenly spaced → CoordRange, monotonic-uneven → CoordMonotonicArray, empty → CoordPartial"), so anything built through it comes out canonical.

The redundancy this PR removes. Every update/select funneled each coord through _get_coord, which did:

coord = coord.model_dump()   # dump a valid coord object to a plain dict
coord = get_coord(**dict)    # ...and rebuild it from scratch

even when coord was already a valid, canonical CoordRange. That round-trip re-runs the full even-spacing scan over the array for nothing — the ~393 µs → ~22 µs (update_coords) and ~596 µs → ~180 µs (value select) cost.

Why we can't skip the rebuild for every coord. That rebuild wasn't only wasted work — for some coords it was silently canonicalizing a coord that had gone stale. Coords go non-canonical via slicing: sample-selection does array indexing (coord[indices]) which keeps the same type and does not re-infer spacing. So:

• [0, 1, 3] → CoordMonotonicArray (uneven, correct)
• sample-select indices 0,1 → subset [0, 1]
• the slice stays a CoordMonotonicArray([0, 1]) — but [0, 1] is now evenly spaced and should be a CoordRange.

The old rebuild fixed that (get_coord([0,1]) → CoordRange). A blanket "skip all BaseCoords" guard would drop the fix-up, leaving a CoordMonotonicArray that wrongly reports evenly_sampled == False — same values, wrong representation. (A second variant: a fully-specified CoordPartial should expand into a CoordRange, else value-based selection on it raises.)

Why CoordRange alone is safe. A CoordRange can never be non-canonical — it is the canonical even form, and slicing a CoordRange yields another CoordRange. So rebuilding one is always pure waste. Every other type might be sitting in a non-canonical state, so those keep going through the rebuild:

if isinstance(coord, CoordRange):
    return coord      # already canonical — no rebuild needed
# all other types fall through to the rebuild, which canonicalizes them

The two regression tests pin exactly the traps that ruled out a broader guard: an even sample-select subset must become a CoordRange, and a fully-specified CoordPartial must become a CoordRange.

TL;DR: the old code rebuilt every coordinate partly to waste time and partly to canonicalize coords that slicing had left in the wrong representation. This skips the rebuild only for CoordRange — the one type guaranteed already-canonical — dropping the wasted work without dropping the fix-up.