Skip to content

refactor: imports cleanup#264

Merged
FlorianDeconinck merged 4 commits into
NOAA-GFDL:developfrom
romanc:romanc/cleanup-imports
Oct 14, 2025
Merged

refactor: imports cleanup#264
FlorianDeconinck merged 4 commits into
NOAA-GFDL:developfrom
romanc:romanc/cleanup-imports

Conversation

@romanc
Copy link
Copy Markdown
Collaborator

@romanc romanc commented Oct 14, 2025

Description

This PR suggests two things related to cleaning up the imports of NDSL.

  1. split GridSizer and SubtileGridSizer
  2. cleanup direct gt4py imports in tests

The reason to do 1) is that GridSizer is used in the QuantityFactory as well as in SubtileGridSizer. Both classes live in the same folder and are exported as public API through the __init__ file. To avoid a circular dependency, I've split out GridSizer in its own module that can be loaded upfront.

The reason for 2) is also historical. By now, we make sure to import . from dsl in the top-level __init__ at the very top. This ensures that we can properly set the literal precision for gt4py in ndsl/dsl/__init__.py.

Alongside, I've ensured no direct imports from mpi4py. I've learned that ndsl.comm.mpi should be the only place to import from mpi4py directly, leaving us the option to easily swap the library in the future (if we then wish to do so).

This PR is motivated by me trying to rebase the stale TracerBundle PR without getting import cycles and having to resolve them by moving random things up and down in __init__ files.

How has this been tested?

All good as long as tests are still green.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation: N/A
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules: N/A
  • New check tests, if applicable, are included: N/A

This PR suggests three things related to cleaning up imports of NDSL.

1. split grid_sizer and subtile_grid_sizer
2. import MPI directly from mpi4py
3. cleanup direct gt4py imports in tests

The reason to do 1) is that `GridSizer` is used in the `QuantityFactory` as
well as in `SubtileGridSizer`. Both classes live in the same folder and are
exported as public API through the `__init__` file. To avoid a circular
dependency, I've split out `GridSizer` in its own module that can be
loaded upfront.

The reason we previsosly imported `MPI` from `ndsl.comm.mpi` was that at
some point `mpi4py` was an optional dependency and `ndsl.comm.mpi` would
export a stub if `mpi4py` wasn't present. Now that `mpi4py` is a
required dependency, we can just get `MPI` from there.

The reason for 3) is also historical. By now, we make sure to `import .
from dsl` in the top-level `__init__` at the very top. This ensures that
we can properly set the literal precision for gt4py in `ndsl/dsl/__init__.py`.
@romanc
Copy link
Copy Markdown
Collaborator Author

romanc commented Oct 14, 2025

Changed the mpi4py stuff back after talking to @FlorianDeconinck who explained that ndsl.comm.mpi is a wrapper around mpi4py by design.

Comment thread tests/dsl/test_stencil_wrapper.py
Co-authored-by: Florian Deconinck <deconinck.florian@gmail.com>
@FlorianDeconinck FlorianDeconinck added this pull request to the merge queue Oct 14, 2025
Merged via the queue into NOAA-GFDL:develop with commit c180927 Oct 14, 2025
5 checks passed
@romanc romanc deleted the romanc/cleanup-imports branch October 14, 2025 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants