Add more tests for rd_region and summary#1260
Open
eivindjahren wants to merge 2 commits into
Open
Conversation
eivindjahren
force-pushed
the
add_more_tests
branch
from
July 24, 2026 13:44
d518b65 to
63f92a5
Compare
There was a problem hiding this comment.
Pull request overview
Adds additional test coverage for resdata.summary.Summary and ResdataRegion selection/keyword operations to better validate edge cases and supported keyword shapes/dtypes.
Changes:
- Add a
Summarytest case ensuring timesteps can be derived fromDAY/MONTH/YEARkeywords whenTIMEis absent. - Expand
ResdataRegiontests to cover polygon “outside” selection, invert behavior, plane selection sizing, and support for active-sized vs global-sized keywords across multiple dtypes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/rd_tests/test_sum.py |
Adds a regression test for reading summary timesteps from DAY/MONTH/YEAR keywords (no TIME). |
tests/rd_tests/test_region.py |
Adds fixtures and tests covering more ResdataRegion selection APIs and keyword size/dtype handling. |
Comments suppressed due to low confidence (2)
tests/rd_tests/test_region.py:481
circumference_polygonandgridare passed as fixtures but not used in the test body (the grid is already accessible viaempty_region). Removing unused fixture parameters keeps the test intent clearer.
def test_that_select_outside_polygon_leaves_only_cells_outside_polygon(
circumference_polygon, empty_region, grid
):
tests/rd_tests/test_region.py:491
circumference_polygonis passed as a fixture but not used in the test body. Removing unused fixture parameters keeps the test intent clearer.
def test_that_deselect_outside_polygon_only_removes_cells_outside_polygon(
circumference_polygon, full_region
):
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+432
to
+436
| # The plane x=5 (normal (1,0,0)) splits the 10x10x1 grid so that the | ||
| # 5x10 slab of cells with i in [0,4] lies strictly below (x<5). | ||
| region = empty_region.copy() | ||
| region.select_below_plane((1.0, 0.0, 0.0), (5.0, 0.0, 0.0)) | ||
| assert region.global_size() == 50 |
Comment on lines
+429
to
+431
| def test_that_select_below_plane_selects_cells_strictly_on_negative_side( | ||
| empty_region, grid | ||
| ): |
MagnusSletten
approved these changes
Jul 24, 2026
MagnusSletten
left a comment
MagnusSletten
left a comment
Contributor
There was a problem hiding this comment.
More tests are always nice! Easy approval from my side. I see copilot noticed a few minor improvements, but those should be trivial to fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.