Conversation
Signed-off-by: Henry Schreiner <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR adds new tests for 2D histograms and refactors existing 1D and 3D indexing tests for consistency. Key changes include the introduction of a new TestAccess2D class with corresponding test methods, the renaming of existing test functions to remove explicit dimension suffixes, and updates to docstrings to reflect variable name changes.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/test_testing.py | Adds the new 2D histogram tests and renames existing test methods for 1D and 3D indexing. |
| src/uhi/testing/indexing.py | Refactors test method names and updates docstrings for clarity across 1D, 2D, and 3D tests. |
Comments suppressed due to low confidence (2)
tests/test_testing.py:543
- [nitpick] In the 3D test 'test_access_loc_mixed', the third index is a literal (9) while the other dimensions use 'self.tag.loc'. Consider using a consistent indexing approach for clarity.
self.assertEqual(self.h[self.tag.loc(0.5), self.tag.loc(4.5), 9], 35)
src/uhi/testing/indexing.py:47
- [nitpick] The renaming of test methods (e.g., from 'test_access_integer_1d' to 'test_access_integer') improves conciseness but may reduce clarity when multiple dimensional tests are present. Consider retaining explicit dimension details to aid in debugging if failures occur.
def test_access_integer(self) -> None:
| This test requires a histogram to be created first. | ||
|
|
||
| h1 is a 1D histogram with 10 bins from 0 to 1. Each bin has 2 more than the | ||
| h is a 1D histogram with 10 bins from 0 to 1. Each bin has 2 more than the |
There was a problem hiding this comment.
[nitpick] The updated docstring now reflects the use of a common variable 'h' instead of dimension-specific names. Ensure that similar naming updates are applied consistently across all histogram test classes for better maintainability.
1 participant
Close #168.