Continuation of #1575, #1603, but here focusing on "from uxarray… import…" statements inside functions.
Overview
Copying relevant notes from PR 1603:
- In some cases, such as in uxarray.core.accessors, code like
from uxarray.core.dataarray import UxDataArray must be kept inside functions, to avoid circular imports.
- There are many cases, such as
UxDataArray.isel containing from uxarray.core.utils import _validate_indexers, where uxarray methods are imported internally without a clear compelling reason to do so. These should probably be moved to top-level. However, the original scope of issue 1575 did not clarify this possibility, and uxarray's explicit non-relative imports make me think it could be an intentional design choice, to reduce "cascading" imports.
More thoughts here:
Are such cases without a "clear compelling reason" actually intentional design decisions? If so, do the benefits outweigh the costs of making the code a bit harder to read and extend?
One example is from uxarray.remap.yac import _yac_remap occurring inside an if statement (which checks if "yac" is being used) inside a function in uxarray/remap/accessor.py, instead of at the top of the file. Moving it to top of file would not cause any circular imports.
Is the idea just to put all use-case-specific imports inside functions, instead of at tops of files? Code from uxarray/grid/grid.py suggests the answer is "no", as there are many use-case-specific imports at the top of the file, such as:
from uxarray.io._exodus import _encode_exodus, _read_exodus
from uxarray.io._fesom2 import _read_fesom2_asci, _read_fesom2_netcdf
from uxarray.io._geopandas import _read_geodataframe
from uxarray.io._geos import _read_geos_cs
from uxarray.io._healpix import _pixels_to_ugrid, _populate_healpix_boundaries
from uxarray.io._icon import _read_icon
...
Proposed changes:
- Relocate all "from uxarray… import…" statements from inside functions to top of file, unless they would cause a circular import or significant performance degradation during
import uxarray. Performance should be measured via ASV benchmarks, paying attention to both the time and peak memory usage of importing uxarray.
- Add comments to clarify any such statements which remain inside functions. The only exception is statements which would obviously cause loops, i.e. there are exactly 2 files involved, and one imports directly from the other. (Example: if fileA contains
from fileB import functionB but functionB needs objectA from fileA, then of course the from fileA import objectA statement must be inside functionB not at top of fileB, so there is no need to explain with comment.)
Continuation of #1575, #1603, but here focusing on "from uxarray… import…" statements inside functions.
Overview
Copying relevant notes from PR 1603:
from uxarray.core.dataarray import UxDataArraymust be kept inside functions, to avoid circular imports.UxDataArray.iselcontainingfrom uxarray.core.utils import _validate_indexers, where uxarray methods are imported internally without a clear compelling reason to do so. These should probably be moved to top-level. However, the original scope of issue 1575 did not clarify this possibility, and uxarray's explicit non-relative imports make me think it could be an intentional design choice, to reduce "cascading" imports.More thoughts here:
Are such cases without a "clear compelling reason" actually intentional design decisions? If so, do the benefits outweigh the costs of making the code a bit harder to read and extend?
One example is
from uxarray.remap.yac import _yac_remapoccurring inside an if statement (which checks if "yac" is being used) inside a function in uxarray/remap/accessor.py, instead of at the top of the file. Moving it to top of file would not cause any circular imports.Is the idea just to put all use-case-specific imports inside functions, instead of at tops of files? Code from uxarray/grid/grid.py suggests the answer is "no", as there are many use-case-specific imports at the top of the file, such as:
Proposed changes:
import uxarray. Performance should be measured via ASV benchmarks, paying attention to both the time and peak memory usage of importing uxarray.from fileB import functionBbutfunctionBneedsobjectAfrom fileA, then of course thefrom fileA import objectAstatement must be insidefunctionBnot at top of fileB, so there is no need to explain with comment.)