Move imports for should-be-optional dependencies#1606
Conversation
such imports are now inside functions instead of at top level. This will support making them optional later in pyproject.toml. This splits off of #1548 to reduce scope of that PR; see that PR for more details.
rajeeja
left a comment
There was a problem hiding this comment.
Reviewed and tested locally — checked out the branch, ran the full suite (565 passed) plus a pre-commit pass, and traced every moved import (ccrs/GeoAxes in dataarray.py/grid.py, pyproj.Geod in sample.py, antimeridian/shapely in geometry.py, healpix in _healpix.py, holoviews in plot/utils.py, scipy.sparse in remap/weights.py) to confirm no runtime usage was left behind outside the function scope it moved into. Clean, well-scoped split from #1548. Approving.
I was wondering if confirmed we had test coverage for all these imports, so I can ask that to you: Did you trace them yourself by eye, or did you confirm test coverage as well? If the latter, I think we can go ahead and merge this; otherwise, I can dod that check. |
|
I'll trace the dependencies and close this out if I find it's working well. |
erogluorhan
left a comment
There was a problem hiding this comment.
This looks great to me! Once I hear back to my question above, I'll approve this.
|
Yeah I don't see anything missing from the stack traces. |
Thanks very much for looking into this, Chris! |
erogluorhan
left a comment
There was a problem hiding this comment.
This is great; thanks Sam!
One step towards solving #1224, but not the full solution.
Overview
Moves imports for "should-be-optional" dependencies to be inside functions instead of at tops of files. All of these changes were originally proposed in PR #1548, which also includes detailed discussion about which dependencies should be optional. However, the changes here do not include edits to pyproject.toml; the should-be-optional dependencies remain fully required, leaving installation processes fully untouched.
The narrower scope of this PR aims to provide simpler-to-accept changes. It was created in response to concerns about the size of PR 1548.
Recommended action plan:
PR Checklist
General