Skip to content

Merge master into dev (#814 review fixes) - #820

Merged
d-chambers merged 2 commits into
devfrom
sync-814-to-dev
Aug 5, 2026
Merged

Merge master into dev (#814 review fixes)#820
d-chambers merged 2 commits into
devfrom
sync-814-to-dev

Conversation

@d-chambers

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

Copy link
Copy Markdown
Contributor

Description

Follow-up sync: brings #814 into dev, which restored review fixes dropped from #808 and #810. Both of those came to dev in #815, so dev inherited the gaps.

The #810 part is a real fix, not cosmetic. Without the len(val) != 2 guard, any collection containing None was treated as a malformed range query. On current dev:

filter_df(df, bp_min=[100, None, 125])
# ParameterError: An open bound (... or None) is not valid in the query for column 'bp_min' ...

That is a legitimate membership query and now behaves as one again. Verified against a reproduction before and after.

docs/tutorial/coords.qmd was the only conflict, because dev rewrote the snap section. Resolved by combining both: dev's description of what snap does and the precision it loses, plus master's warning that snap returns no indexer, so an unsorted coordinate's snapped values no longer label the samples they used to. Both CoordManager.snap and Patch.snap_coords are referenced for the alignment case; all three link targets verified to exist on dev.

Changelog

none

Checklist

I have (if applicable):

  • referenced the GitHub issue this PR closes. (No issue; keeps dev in sync with master.)
  • 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

    • Corrected filtering for one- and three-value collections containing None, ensuring they perform membership matching rather than range filtering.
  • Documentation

    • Clarified that snapping individual coordinates returns a new coordinate only and may misalign unsorted labels with their data.
    • Added guidance to use higher-level snapping tools when coordinates and associated data must remain aligned.
  • Tests

    • Added coverage for None values in membership filters.

d-chambers and others added 2 commits August 4, 2026 21:16
Brings #814, which restores review fixes dropped from #808 and #810.

The #810 fix matters: without the `len(val) != 2` guard, a collection
like [100, None, 125] was read as a malformed range and raised instead
of doing a membership check.

docs/tutorial/coords.qmd conflicts because dev rewrote the snap
section. Kept dev's description of what snap does and its precision
loss, and folded in master's warning that snap returns no indexer, so
an unsorted coordinate's snapped values no longer label the same
samples.
@d-chambers d-chambers added documentation Improvements or additions to documentation ready_for_review PR is ready for review labels Aug 5, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0a5f2f31-fdc2-457f-9cf0-7175949bfd89

📥 Commits

Reviewing files that changed from the base of the PR and between 4536276 and 7aa9f6d.

📒 Files selected for processing (3)
  • dascore/utils/pd.py
  • docs/tutorial/coords.qmd
  • tests/test_utils/test_pd.py

📝 Walkthrough

Walkthrough

The query helper now treats only two-element collections as ranges. Other collections use membership matching, including None. Tests cover this behavior. The coordinate snapping documentation clarifies BaseCoord.snap behavior and references data-preserving alternatives.

Changes

Query Matching and Snap Documentation

Layer / File(s) Summary
Membership filtering and validation
dascore/utils/pd.py, tests/test_utils/test_pd.py
Range validation now applies only to two-element collections. Parametrized tests cover one-element and three-element collections containing None.
Coordinate snapping documentation
docs/tutorial/coords.qmd
The documentation clarifies BaseCoord.snap behavior and references CoordManager.snap and Patch.snap_coords for coordinated data snapping.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: syncing master into dev to restore review fixes from issue #814.
Description check ✅ Passed The description explains the sync, code fix, documentation update, tests, and checklist status in sufficient detail.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync-814-to-dev

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.

@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (4536276) to head (7aa9f6d).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff            @@
##               dev      #820   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          164       164           
  Lines        17879     17879           
=========================================
  Hits         17879     17879           
Flag Coverage Δ
network 48.42% <0.00%> (ø)
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.

@d-chambers
d-chambers merged commit 732d5b9 into dev Aug 5, 2026
34 checks passed
@d-chambers
d-chambers deleted the sync-814-to-dev branch August 5, 2026 05:18
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

✅ Documentation built:
👉 Download
Note: You must be logged in to github and a DASDAE member to access the link.

@d-chambers d-chambers removed the ready_for_review PR is ready for review label Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant