Base class for IO test files #1331

rajeeja · 2025-08-05T23:49:36Z

#1329 better performance and more maintainable, improved coverage.

…asset :), keeping GeoPandas, Structured and HEALPix as they are different and just file format reading/writing, geopandas uses Grid.from_file() unlike others

philipc2

This looks great. I like the idea of centralized tests for all grid formats.

Though, I still would like to have separate modules (i.e. test_mpas) instead of putting all in one module.

Maybe something like the following structure?

Create a base set of tests we can run on all grid formats (similar to what you've done), which can be imported and used within each of the individual modules. This would still allow us to add extra tests via inheritance if needed.

…array into rajeeja/io_test_consolidation

rajeeja · 2025-08-06T16:16:46Z

This looks great. I like the idea of centralized tests for all grid formats.

Though, I still would like to have separate modules (i.e. test_mpas) instead of putting all in one module.

Maybe something like the following structure?

Create a base set of tests we can run on all grid formats (similar to what you've done), which can be imported and used within each of the individual modules. This would still allow us to add extra tests via inheritance if needed.

I have made this change, but I like the one-module design is better. It is typically the case that one reader-writer breaks the others, and we don't test it. If needed, we can always run a part of the tests from that one module.

philipc2 · 2025-08-06T17:00:22Z

I have made this change, but I like the one-module design is better.

I'm open to having a one-module design, especially one that tests basic functionality like you've shown, including

Reading and Writing Grids
Checking UGRID compliance.

However, for grid-specific tests (i.e. HEALPix or MPAS), it still makes sense to me to keep the existing modules, which would test extra functionality on top of the standard set in the suite you created.

- Revert format-specific test files to original state - Add new test_io_common.py with common IO tests across all formats - Remove complex inheritance structure - Keep flat test structure as requested - Tests cover: basic read, UGRID compliance, error handling, lazy loading - All format-specific tests remain in their original files

rajeeja · 2025-08-14T04:13:59Z

adding only test_io_common.py for shared IO functionality while keeping all format-specific tests unchanged. We should create a follow-up issue to reorganize the entire test suite into a hierarchical structure (mirroring uxarray's module organization), breaking down large files like test_geometry.py (60KB) into smaller, more manageable components.

test/test_io_common.py

rajeeja · 2025-08-14T21:42:59Z

Thanks for the detailed review and comments. I have tried to keep the changes minimal in this pull request, addressed most of your comments. There might be some duplicated tests in individual I/O modules and this new test. But IMO that's okay for now. I think we can first do a complete reorganization of the test suite by folders and modules and let this pull request go first.

test/test_io_common.py

Co-authored-by: Philip Chmielowiec <67855069+philipc2@users.noreply.github.com>

…ormats to write

philipc2 · 2025-08-18T13:31:15Z

test/test_io_common.py

+IO_READ_TEST_FORMATS = [
+    ("ugrid", "ugrid/quad-hexagon", "grid.nc"),
+    ("ugrid", "ugrid/outCSne30", "outCSne30.ug"),
+    ("ugrid", "ugrid/outRLL1deg", "outRLL1deg.ug"),
+    ("mpas", "mpas/QU/480", "grid.nc"),
+    ("esmf", "esmf/ne30", "ne30pg3.grid.nc"),
+    ("exodus", "exodus/outCSne8", "outCSne8.g"),
+    ("exodus", "exodus/mixed", "mixed.exo"),
+    ("scrip", "scrip/outCSne8", "outCSne8.nc"),
+    ("icon", "icon/R02B04", "icon_grid_0010_R02B04_G.nc"),
+    ("fesom", "fesom/pi", None),  # Special case - multiple files
+]


Could we include HEALPix in here?

There's no grid file associated with it, but within our fixture below we can construct a basic one by doing:

grid = ux.Grid.from_healpix(zoom=1, pixels_only=False)

test/test_io_common.py

Co-authored-by: Philip Chmielowiec <67855069+philipc2@users.noreply.github.com>

…from_healpix to IO_READ_TEST_FORMATS fixture; refactor tests to use fixture; check properties via grid.connectivity/coordinates; use np.issubdtype for dtype checks.

test/test_io_common.py

philipc2 · 2025-08-18T17:11:48Z

test/test_io_common.py

+try:
+    import constants
+except ImportError:
+    from . import constants


@rajeeja

Can we remove this? I don't think is doing anything right now.

test/test_io_common.py

philipc2 · 2025-08-18T17:13:13Z

test/test_io_common.py

+    def test_lazy_loading(self, grid_from_format):
+        """Test that grids support lazy loading where applicable."""
+        grid = grid_from_format
+
+        assert grid._ds is not None


@rajeeja

Can we remove this test?

test/test_io_common.py

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

test/test_io_common.py

Co-authored-by: Philip Chmielowiec <67855069+philipc2@users.noreply.github.com>

test/test_io_common.py

philipc2

Other than my other comment, this looks great!

…ssert)

o Transform IO testing from a maintenance burden into a professional …

21ffed6

…asset :), keeping GeoPandas, Structured and HEALPix as they are different and just file format reading/writing, geopandas uses Grid.from_file() unlike others

rajeeja requested review from erogluorhan and philipc2 August 5, 2025 23:49

philipc2 requested changes Aug 6, 2025

View reviewed changes

philipc2 assigned rajeeja Aug 6, 2025

philipc2 linked an issue Aug 6, 2025 that may be closed by this pull request

IO Test Overhaul #1329

Closed

rajeeja added 4 commits August 6, 2025 01:34

o Get theio testing files back with base tests etc.

e69ca9a

o Precommit

5e4d281

o Add missing base tests file

ffd0052

Merge branch 'rajeeja/io_test_consolidation' of github.com:UXARRAY/ux…

c8ba890

…array into rajeeja/io_test_consolidation

rajeeja changed the title ~~Replace 7 scattered IO test files with unified test_io~~ Base class for IO test files Aug 6, 2025

Rajeev Jain and others added 3 commits August 13, 2025 23:03

o whitwspace

9300746

Merge branch 'main' into rajeeja/io_test_consolidation

5397b20

rajeeja requested a review from philipc2 August 14, 2025 04:14

philipc2 requested changes Aug 14, 2025

View reviewed changes

philipc2 reviewed Aug 14, 2025

View reviewed changes

test/test_io_common.py Show resolved Hide resolved

Rajeev Jain added 2 commits August 14, 2025 16:39

Revert pyproject.toml changes - remove pytest configuration

89566de

o Address feedback comments use IO_READ_TEST_FORMATS

6ce5c35

philipc2 requested changes Aug 15, 2025

View reviewed changes

philipc2 reviewed Aug 15, 2025

View reviewed changes

test/test_io_common.py Outdated Show resolved Hide resolved

rajeeja requested a review from philipc2 August 15, 2025 22:34

rajeeja and others added 2 commits August 15, 2025 17:40

Update test/test_io_common.py

3dae07a

Co-authored-by: Philip Chmielowiec <67855069+philipc2@users.noreply.github.com>

o Remove this TestIODatasetCommon class and focus on grid. Add more f…

c9a11c0

…ormats to write

philipc2 requested changes Aug 18, 2025

View reviewed changes

Update test/test_io_common.py

decc85b

Co-authored-by: Philip Chmielowiec <67855069+philipc2@users.noreply.github.com>

rajeeja and others added 5 commits August 18, 2025 10:16

Update test/test_io_common.py

c8d0191

Co-authored-by: Philip Chmielowiec <67855069+philipc2@users.noreply.github.com>

Update test/test_io_common.py

1d26c72

Co-authored-by: Philip Chmielowiec <67855069+philipc2@users.noreply.github.com>

Update test/test_io_common.py

8a75fc9

Co-authored-by: Philip Chmielowiec <67855069+philipc2@users.noreply.github.com>

Update test/test_io_common.py

e3d4f0f

Co-authored-by: Philip Chmielowiec <67855069+philipc2@users.noreply.github.com>

tests/io_common: address review feedback – add HEALPix case via Grid.…

984d3e1

…from_healpix to IO_READ_TEST_FORMATS fixture; refactor tests to use fixture; check properties via grid.connectivity/coordinates; use np.issubdtype for dtype checks.

philipc2 requested changes Aug 18, 2025

View reviewed changes

test/test_io_common.py Outdated Show resolved Hide resolved

test/test_io_common.py Outdated Show resolved Hide resolved

o Use grid.dims and create a new function test_ugrid_compliance

432f4cb

philipc2 requested changes Aug 18, 2025

View reviewed changes

philipc2 reviewed Aug 18, 2025

View reviewed changes

test/test_io_common.py Outdated Show resolved Hide resolved

Rajeev Jain and others added 2 commits August 18, 2025 14:15

o tests/io_common: clarify docstrings for cross-format IO tests

97a918f

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

o pre-commit: auto-format test_io_common.py

83b6625

philipc2 reviewed Aug 18, 2025

View reviewed changes

test/test_io_common.py Outdated Show resolved Hide resolved

rajeeja and others added 4 commits August 18, 2025 18:11

Update test/test_io_common.py

7e81eea

Co-authored-by: Philip Chmielowiec <67855069+philipc2@users.noreply.github.com>

o Fix import and asserts

2af329b

o precommit

6aa93ae

Merge branch 'main' into rajeeja/io_test_consolidation

bcf14a1

philipc2 reviewed Aug 19, 2025

View reviewed changes

test/test_io_common.py Outdated Show resolved Hide resolved

philipc2 approved these changes Aug 19, 2025

View reviewed changes

Rajeev Jain added 2 commits August 19, 2025 14:54

o Add assert and check warnings

30307a6

test(io): align validate() assertion style with other tests (simple a…

8fbe592

…ssert)

rajeeja merged commit 2e68c1a into main Aug 20, 2025
19 of 20 checks passed

erogluorhan deleted the rajeeja/io_test_consolidation branch September 26, 2025 17:47

Base class for IO test files #1331

Base class for IO test files #1331

Uh oh!

Conversation

rajeeja commented Aug 5, 2025

Uh oh!

philipc2 left a comment

Choose a reason for hiding this comment

Uh oh!

rajeeja commented Aug 6, 2025

Uh oh!

philipc2 commented Aug 6, 2025

Uh oh!

rajeeja commented Aug 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rajeeja commented Aug 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philipc2 Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philipc2 Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

philipc2 Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philipc2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants