Reduce imports inside functions

[Sevans711]

Closes#1575

Overview

There are some places throughout the uxarray codebase where import is being used inside functions. This PR targets imports which are not "heavy" or which are already being imported non-lazily (i.e., at they are imported at "top-level" / top of file), changing them to be consistently imported non-lazily everywhere in uxarray.

Notes about all packages being imported internally in at least one location (e.g. imported inside a function; not just at top of file):

• antimeridian: not currently at top-level anywhere. Tiny required dependency, seems trivially-lightweight to import. But, might become optional some day...? → keep internal.
• dask: not currently at top-level anywhere, and not trivially-lightweight to import → keep internal
• datetime: trivially lightweight and builtin, no need to import lazily → move to top.
• IPython: not currently at top-level anywhere; optional (but forgot to list in pyproject.toml at all), and will probably be removed eventually, whenever upstream fix is released (see discussion in Restore matplotlib backend after HoloViews matplotlib plot #1538 and upstream issue at hv.extension('matplotlib') clobbers active Matplotlib backend and IPython inline display integration holoviz/holoviews#6955) → do not touch in this PR.
• numpy: essential for core functionality and definitions, already at top level in many places → move to top level.
• pyfma: only appears in an otherwise-unused module? (See uxarray/utils/computing.py not used anywhere, and pyfma not in pyproject.toml #1602) → do not touch in this PR.
• scipy: non-trivial import size, but does exist as top-level import in weights.py, which makes this a bit ambiguous. → did not touch here, but open for further discussion (should the scipy import in weights.py be moved to be internal instead?). EDIT: this would be fully addressed by Move imports for should-be-optional dependencies #1606, which moves that scipy import to be internal instead → do not touch in this PR.
• sklearn: not currently at top-level anywhere; non-trivial import size → keep internal.
• shapely: same story as with antimeridian: not currently at top-level anywhere. Tiny required dependency, seems trivially-lightweight to import. But, might become optional some day...? → keep internal.
• warnings: trivially lightweight and builtin, no need to import lazily → move to top.
• xarray: essential for core functionality and definitions, already at top level in many places → move to top level.
• Many packages should be an optional dependency eventually/soon (see Refactor dependencies to make more optional #1548) → do not touch in this PR; these are already handled by that PR (or maybe will be moved to a separate issue/PR). This applies to:
  • cartopy
  • geopandas
  • holoviews
  • hvplot
  • matplotlib
  • spatialpandas

Notes about importing from uxarray internally to functions:

• 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 Reduce number of imports inside functions #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. I think it makes more sense to open a separate issue to discuss internal imports like "from uxarray... import ...", instead of trying to also consider that here. If PR reviewers agree, let's open that issue before this PR gets merged

Misc notes:

• In some cases, imports were gated by "if TYPE_CHECKING" unnecessarily, such as "if TYPE_CHECKING: from xarray import Dataset". There is no reason to do this when xarray is already imported in the file → cleaned up cases like this.

PR Checklist

General

• An issue is linked created and linked
• Add appropriate labels
• Filled out Overview and Expected Usage (if applicable) sections

[erogluorhan]

The work here seems great to me! Please make sure all the tests are passing. I will get back to your discussion points and approve this then

[dylannelson]

The notes and comments here helped a lot, appreciate it! Thanks!

[github-actions]

ASV Benchmarking

Benchmark Comparison Results

Benchmarks that have improved:

Change	Before [71d6e37]	After [33cb5a5]	Ratio	Benchmark (Parameter)
-	578M	391M	0.68	face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/geoflow-small/grid.nc'))
-	708M	390M	0.55	face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/quad-hexagon/grid.nc'))
-	498M	384M	0.77	mpas_ocean.Gradient.peakmem_gradient('480km')

Benchmarks that have stayed the same:

Change	Before [71d6e37]	After [33cb5a5]	Ratio	Benchmark (Parameter)
	10.6±0.2μs	10.8±0.1μs	1.02	bench_connectivity.Connectivity.time_edge_face('120km')
	11.1±0.2μs	12.6±1μs	~1.13	bench_connectivity.Connectivity.time_edge_face('480km')
	10.5±0.1μs	10.5±0.1μs	1.00	bench_connectivity.Connectivity.time_edge_node('120km')
	11.2±0.1μs	10.8±0.07μs	0.97	bench_connectivity.Connectivity.time_edge_node('480km')
	10.6±0.2μs	10.6±0.08μs	1.00	bench_connectivity.Connectivity.time_face_edge('120km')
	11.1±0.07μs	10.9±0.2μs	0.98	bench_connectivity.Connectivity.time_face_edge('480km')
	10.6±0.1μs	10.5±0.2μs	0.99	bench_connectivity.Connectivity.time_face_face('120km')
	10.9±0.1μs	11.0±0.2μs	1.01	bench_connectivity.Connectivity.time_face_face('480km')
	21.5±0.2μs	21.5±0.1μs	1.00	bench_connectivity.Connectivity.time_face_node('120km')
	22.5±0.3μs	22.5±0.3μs	1.00	bench_connectivity.Connectivity.time_face_node('480km')
	10.5±0.3μs	10.6±0.1μs	1.01	bench_connectivity.Connectivity.time_node_edge('120km')
	11.0±0.1μs	11.0±0.2μs	0.99	bench_connectivity.Connectivity.time_node_edge('480km')
	10.6±0.07μs	10.7±0.08μs	1.01	bench_connectivity.Connectivity.time_node_face('120km')
	11.2±0.08μs	10.8±0.2μs	0.97	bench_connectivity.Connectivity.time_node_face('480km')
	389M	389M	1.00	face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/mpas/QU/oQU480.231010.nc'))
	419M	419M	1.00	face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/scrip/outCSne8/outCSne8.nc'))
	15.1±0.3ms	15.3±0.1ms	1.01	face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/mpas/QU/oQU480.231010.nc'))
	3.94±0.2ms	4.00±0.06ms	1.01	face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/scrip/outCSne8/outCSne8.nc'))
	19.2±0.04ms	19.1±0.2ms	1.00	face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/geoflow-small/grid.nc'))
	2.19±0.01ms	2.19±0.02ms	1.00	face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/quad-hexagon/grid.nc'))
	1.06±0s	1.03±0.01s	0.97	import.Imports.timeraw_import_uxarray
	930±3ns	924±7ns	0.99	mpas_ocean.CheckNorm.time_check_norm('120km')
	887±2ns	886±10ns	1.00	mpas_ocean.CheckNorm.time_check_norm('480km')
	845±4ms	840±5ms	0.99	mpas_ocean.ConnectivityConstruction.time_face_face_connectivity('120km')
	54.3±0.4ms	53.4±0.3ms	0.98	mpas_ocean.ConnectivityConstruction.time_face_face_connectivity('480km')
	704±10μs	693±8μs	0.98	mpas_ocean.ConnectivityConstruction.time_n_nodes_per_face('120km')
	612±10μs	592±20μs	0.97	mpas_ocean.ConnectivityConstruction.time_n_nodes_per_face('480km')
	5.45±0.02ms	5.44±0.03ms	1.00	mpas_ocean.ConstructFaceLatLon.time_cartesian_averaging('120km')
	4.00±0.02ms	3.92±0.04ms	0.98	mpas_ocean.ConstructFaceLatLon.time_cartesian_averaging('480km')
	3.45±0.02s	3.43±0.03s	0.99	mpas_ocean.ConstructFaceLatLon.time_welzl('120km')
	225±2ms	225±3ms	1.00	mpas_ocean.ConstructFaceLatLon.time_welzl('480km')
	18.2±0.01ms	18.2±0.03ms	1.00	mpas_ocean.ConstructTreeStructures.time_ball_tree('120km')
	1.08±0.01ms	1.07±0.01ms	1.00	mpas_ocean.ConstructTreeStructures.time_ball_tree('480km')
	10.7±0.02ms	10.6±0.02ms	1.00	mpas_ocean.ConstructTreeStructures.time_kd_tree('120km')
	725±10μs	727±10μs	1.00	mpas_ocean.ConstructTreeStructures.time_kd_tree('480km')
	706±9ms	710±20ms	1.01	mpas_ocean.CrossSections.time_const_lat('120km', 1)
	358±5ms	366±2ms	1.02	mpas_ocean.CrossSections.time_const_lat('120km', 2)
	185±2ms	189±2ms	1.02	mpas_ocean.CrossSections.time_const_lat('120km', 4)
	550±3ms	556±10ms	1.01	mpas_ocean.CrossSections.time_const_lat('480km', 1)
	279±3ms	279±4ms	1.00	mpas_ocean.CrossSections.time_const_lat('480km', 2)
	144±2ms	144±3ms	1.00	mpas_ocean.CrossSections.time_const_lat('480km', 4)
	25.0±0.3ms	25.1±0.1ms	1.01	mpas_ocean.DualMesh.time_dual_mesh_construction('120km')
	3.65±0.3ms	3.72±0.3ms	1.02	mpas_ocean.DualMesh.time_dual_mesh_construction('480km')
	958±10ms	951±5ms	0.99	mpas_ocean.GeoDataFrame.time_to_geodataframe('120km', False)
	54.2±1ms	51.8±0.8ms	0.96	mpas_ocean.GeoDataFrame.time_to_geodataframe('120km', True)
	84.7±0.7ms	85.5±1ms	1.01	mpas_ocean.GeoDataFrame.time_to_geodataframe('480km', False)
	5.96±0.1ms	5.90±0.1ms	0.99	mpas_ocean.GeoDataFrame.time_to_geodataframe('480km', True)
	404M	404M	1.00	mpas_ocean.Gradient.peakmem_gradient('120km')
	175±0.6ms	176±0.8ms	1.00	mpas_ocean.Gradient.time_gradient('120km')
	12.6±0.2ms	12.9±0.01ms	1.03	mpas_ocean.Gradient.time_gradient('480km')
	226±3μs	227±1μs	1.00	mpas_ocean.HoleEdgeIndices.time_construct_hole_edge_indices('120km')
	133±0.9μs	134±1μs	1.01	mpas_ocean.HoleEdgeIndices.time_construct_hole_edge_indices('480km')
	356M	351M	0.99	mpas_ocean.Integrate.peakmem_integrate('120km')
	330M	331M	1.00	mpas_ocean.Integrate.peakmem_integrate('480km')
	222±1μs	220±2μs	0.99	mpas_ocean.Integrate.time_integrate('120km')
	200±1μs	199±0.7μs	0.99	mpas_ocean.Integrate.time_integrate('480km')
	182±1ms	185±1ms	1.01	mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'exclude')
	184±1ms	184±2ms	1.00	mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'include')
	184±1ms	185±0.9ms	1.01	mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'split')
	13.7±0.06ms	14.1±0.3ms	1.03	mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'exclude')
	13.6±0.2ms	14.2±0.6ms	1.05	mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'include')
	13.5±0.3ms	13.9±0.09ms	1.03	mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'split')
	355±0.9μs	356±4μs	1.00	mpas_ocean.PointInPolygon.time_face_search_lonlat('120km')
	362±4μs	358±2μs	0.99	mpas_ocean.PointInPolygon.time_face_search_lonlat('480km')
	337±6μs	340±2μs	1.01	mpas_ocean.PointInPolygon.time_face_search_xyz('120km')
	340±5μs	338±4μs	1.00	mpas_ocean.PointInPolygon.time_face_search_xyz('480km')
	246±0.1ms	251±0.8ms	1.02	mpas_ocean.RemapDownsample.time_bilinear_remapping
	300±1ms	300±1ms	1.00	mpas_ocean.RemapDownsample.time_inverse_distance_weighted_remapping
	4.87±0.06ms	4.89±0.06ms	1.00	mpas_ocean.RemapDownsample.time_nearest_neighbor_remapping
	1.45±0.01s	1.46±0.02s	1.00	mpas_ocean.RemapUpsample.time_bilinear_remapping
	37.2±0.8ms	37.3±0.5ms	1.00	mpas_ocean.RemapUpsample.time_inverse_distance_weighted_remapping
	9.85±0.1ms	9.74±0.08ms	0.99	mpas_ocean.RemapUpsample.time_nearest_neighbor_remapping
	29.4±0.8ms	29.5±0.9ms	1.00	mpas_ocean.ZonalAverage.time_zonal_average('120km')
	7.20±0.06ms	7.26±0.1ms	1.01	mpas_ocean.ZonalAverage.time_zonal_average('480km')
	327M	330M	1.01	quad_hexagon.QuadHexagon.peakmem_open_dataset
	326M	325M	1.00	quad_hexagon.QuadHexagon.peakmem_open_grid
	7.61±0.2ms	7.11±0.3ms	0.93	quad_hexagon.QuadHexagon.time_open_dataset
	6.18±0.3ms	6.14±0.3ms	0.99	quad_hexagon.QuadHexagon.time_open_grid

[Sevans711]

Thank you both for taking a look!

Re: @erogluorhan's point about tests:

Please make sure all the tests are passing. I will get back to your discussion points and approve this then

All the tests do seem to pass with the code here, I manually triggered a CI run on this branch (https://github.com/UXARRAY/uxarray/actions/runs/29865868247), after the original run failed one test just due to a network error (link for posterity: https://github.com/UXARRAY/uxarray/actions/runs/29864267121/job/88748409432?pr=1603). This rerun involved no code changes. I also opened #1604 for further discussion about network failures in the CI autotests.

What is the preferred workflow when this happens? I wasn't able to find a "rerun tests directly in the PR interface" button. One option: I can push a commit with basically no changes (e.g., comment at the single top-level scipy import to say "maybe this should move to be not at top-level? see PR # 1603 for more details"). I will wait for your response before pushing that though, to help the current issue remain clear until then.

[Sevans711]

Tests now all passed successfully, thank you for showing me where the "rerun failed jobs in this PR" button lives, @erogluorhan!

I suspect I may have failed to find it before due to looking too early, before all the jobs had finished? Or, maybe I simply didn't know where to look. I left this comment about it: #1604 (comment) for future reference.