Skip to content

test: add test for uncompacting to a coarser resolution - #497

Open
dyrpsf wants to merge 3 commits into
uber:masterfrom
dyrpsf:test-uncompact-resolution
Open

test: add test for uncompacting to a coarser resolution#497
dyrpsf wants to merge 3 commits into
uber:masterfrom
dyrpsf:test-uncompact-resolution

Conversation

@dyrpsf

@dyrpsf dyrpsf commented Jul 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This PR resolves a TODO comment left in uncompact_cells regarding missing test coverage.

I removed the TODO from src\h3\api\basic_int\__init__.py and added a test in tests/test_lib/test_cells_and_edges.py to verify that H3ResMismatchError is correctly raised when a user attempts to uncompact a cell into a coarser resolution.

Maintainers

cc: @dfellis @ajfriend

@dyrpsf
dyrpsf force-pushed the test-uncompact-resolution branch from 6c5f9e9 to 2f62194 Compare July 29, 2026 11:40
@dyrpsf

dyrpsf commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

just a quick heads-up: the CI caught a couple of minor formatting issues from the linter (just a missing blank line and a trailing space in the test file). I've fixed the spacing and force-pushed the update. Everything should be perfectly green now!

isaacbrodsky
isaacbrodsky requested changes Jul 29, 2026
View reviewed changes
Comment thread tests/test_lib/test_cells_and_edges.py Outdated
@dyrpsf @isaacbrodsky
Update tests/test_lib/test_cells_and_edges.py
58be699 
Co-authored-by: Isaac Brodsky <isaac@isaacbrodsky.com>
@dyrpsf

dyrpsf commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

@isaacbrodsky Thanks for catching that! I've applied your suggestion to use H3ResMismatchError. Could you or @dfellis please approve the workflow run so the CI can process the new commit? Thanks!

isaacbrodsky
isaacbrodsky reviewed Aug 3, 2026
View reviewed changes
Comment thread tests/test_lib/test_cells_and_edges.py Outdated
@dyrpsf

dyrpsf commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Great catch! I updated the code to use H3ResMismatchError based on the earlier review, but I completely missed updating the inline comment and the PR description to match. I've just pushed a commit to fix the comment, and I updated the PR description as well so everything is consistent. Thanks for keeping an eye on that!

@dyrpsf
dyrpsf requested a review from isaacbrodsky August 3, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@isaacbrodsky isaacbrodsky Awaiting requested review from isaacbrodsky

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dyrpsf @isaacbrodsky