private coords to not affect equality #586

d-chambers · 2025-12-03T01:40:48Z

Description

Private coordinates should not affect equality checks in coord managers. This PR tests and enforces this. Now coords behave the same as attributes.

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
- Improved equality comparison logic for coordinate objects to properly handle private coordinates and enforce dimension alignment checks.
Tests
- Added test coverage for private coordinate handling in equality comparisons and model equality with disjoint private attributes.

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

coderabbitai · 2025-12-03T01:40:57Z

Walkthrough

Modified the CoordManager equals method to properly handle private coordinates by comparing dimensions and filtering private coordinates before per-coordinate comparisons. Added corresponding test cases to verify private coordinate behavior in equality checks across CoordManager and model utilities.

Changes

Cohort / File(s)	Change Summary
CoordManager Core Implementation `dascore/core/coordmanager.py`	Updated `equals` method to compare dimensions explicitly and accommodate private coordinates by building sets of public and non-private coordinate names; private coordinates no longer require exact equality, and dimension alignment is enforced prior to coordinate-wise comparisons.
CoordManager Tests `tests/test_core/test_coordmanager.py`	Added two test methods: `test_private_non_dimension_coord` verifies private non-dimensional coordinates don't affect CoordManager equality; `test_private_dims_effect_equals` verifies private coordinate names shadowing dimensions impact equality.
Model Utility Tests `tests/test_utils/test_models.py`	Added `test_private_disjoint` test method to TestModelEquals to verify that private attributes with non-overlapping names don't affect equality comparisons in sensible_model_equals.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: making private coordinates not affect equality comparisons.
Description check	✅ Passed	The description explains the purpose (private coords should not affect equality), mentions testing and enforcement, and references the alignment with attribute behavior. However, no GitHub issue is referenced despite the template asking for it.
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 ignore_private_in_compare

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between efca919 and 458f996.

📒 Files selected for processing (3)

dascore/core/coordmanager.py (1 hunks)
tests/test_core/test_coordmanager.py (1 hunks)
tests/test_utils/test_models.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

tests/test_utils/test_models.py (1)

dascore/utils/models.py (1)

sensible_model_equals (68-85)

tests/test_core/test_coordmanager.py (4)

tests/conftest.py (1)

cm_basic (106-108)

dascore/core/coordmanager.py (3)

get_array (1077-1079)

new (453-471)

update (240-308)

dascore/core/coords.py (3)

new (592-603)

update (742-755)

update (983-985)

dascore/core/attrs.py (1)

update (234-255)

dascore/core/coordmanager.py (1)

dascore/core/patch.py (1)

dims (212-226)

🔇 Additional comments (4)

tests/test_utils/test_models.py (1)

35-39: LGTM! Test correctly verifies disjoint private attributes don't affect equality.

The test validates that models with different private attribute names are still considered equal by sensible_model_equals, since that function skips all attributes starting with "_" during comparison.

dascore/core/coordmanager.py (1)

782-793: LGTM! Private coordinate filtering correctly implemented.

The updated equality logic properly handles private coordinates:

Dimensions are compared first for early exit

Private non-dimensional coordinates are excluded from comparison (lines 785-786)

Private dimensional coordinates still affect equality (included via x in self.dims condition)

This aligns with the PR objective where private coords behave like private attributes.

tests/test_core/test_coordmanager.py (2)

825-829: LGTM! Test correctly verifies private non-dimensional coordinates are ignored.

The test confirms that adding a private coordinate (name starts with "_") that is not a dimension does not affect equality between CoordManager instances. This aligns with the PR objective to make coords behave like attrs.

831-841: LGTM! Test correctly verifies private dimensions still affect equality.

The test confirms that:

CoordManagers with different coordinate values are not equal (lines 834-836)

Renaming a dimension to a private name (starts with "_") doesn't change this behavior - they remain unequal (lines 838-841)

This correctly validates that private dimensional coordinates still affect equality, as they should since dimensions define the structure of the data.

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-03T01:44:11Z

Codecov Report

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

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #586   +/-   ##
=======================================
  Coverage   99.92%   99.92%           
=======================================
  Files         129      129           
  Lines       11237    11241    +4     
=======================================
+ Hits        11229    11233    +4     
  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.

codspeed-hq · 2025-12-03T02:13:02Z

CodSpeed Performance Report

Merging #586 will not alter performance

_{Comparing ignore_private_in_compare (458f996) with master (efca919)}

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. ↩

private coords to not affect equals

458f996

d-chambers merged commit 1dccf58 into master Dec 3, 2025
23 checks passed

d-chambers deleted the ignore_private_in_compare branch December 3, 2025 03:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

private coords to not affect equality #586

private coords to not affect equality #586

Uh oh!

d-chambers commented Dec 3, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 3, 2025 •

edited

Loading

Uh oh!

codecov bot commented Dec 3, 2025 •

edited

Loading

Uh oh!

codspeed-hq bot commented Dec 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

private coords to not affect equality #586

private coords to not affect equality #586

Uh oh!

Conversation

d-chambers commented Dec 3, 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 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Pre-merge checks and finishing touches

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

codspeed-hq bot commented Dec 3, 2025

CodSpeed Performance Report

Merging #586 will not alter performance

Summary

Footnotes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

d-chambers commented Dec 3, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 3, 2025 •

edited

Loading

codecov bot commented Dec 3, 2025 •

edited

Loading