Skip to content

fix(thermoelastic3d): import napari lazily #260

Open
cashend wants to merge 1 commit into
mainfrom
fix/thermoelastic3d-lazy-napari
Open

fix(thermoelastic3d): import napari lazily #260
cashend wants to merge 1 commit into
mainfrom
fix/thermoelastic3d-lazy-napari

Conversation

@cashend

@cashend cashend commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Description

thermoelastic3d/v0.py and thermoelastic3d/model/fem_plotting.py imported napari at module
top level. Since napari is an optional dependency (declared under the thermoelastic3d/all
extras and used only for 3D rendering), importing the problem — and therefore collecting
tests/test_problem_implementations.py. Fails with ModuleNotFoundError: No module named 'napari'
in any environment without napari installed.

Fix: import napari lazily inside the render/plot functions that use it. The module now imports
without napari present; napari is only required when actually rendering. No behavior change when
napari is installed.

Fixes # (no tracked issue; create one if desired)

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

N/A.

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files
  • I have run ruff check . and ruff format
  • I have run mypy .
  • I have commented my code, particularly in hard-to-understand areas (each lazy import carries a # noqa: PLC0415 comment explaining why)
  • I have made corresponding changes to the documentation — N/A
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective — no new test added, but test_problem_implementations.py now collects and passes without napari, which previously errored
  • New and existing unit tests pass locally with my changes (thermoelastic3d problem tests pass; collection succeeds without napari)

Reviewer Checklist (pre-assessed — confirm during review):

  • Brings value / not too specific — optional dep no longer breaks import/collection
  • Tests/checks pass (lint, format, type)
  • Documentation updated — N/A
  • Code understandable and commented
  • No merge conflict (branch cleanly off latest main)
  • Not breaking existing results; no version bump needed (lazy import only)
  • New-problem dataset via slurm — N/A
  • Robust fix, not a hacky workaround (standard lazy-import pattern for optional deps)

@markfuge

Copy link
Copy Markdown
Member

@cashend and @arthurdrake1
Meta question: do we even need napari as a library at all? I understand Vic used this, and perhaps it is helpful in certain circumstances, but is it really worth the additional dependency, if it is only used for Rendering? Could similar functionality be gained using an existing visualization dependency?

@arthurdrake1

Copy link
Copy Markdown
Contributor

@cashend and @arthurdrake1 Meta question: do we even need napari as a library at all? I understand Vic used this, and perhaps it is helpful in certain circumstances, but is it really worth the additional dependency, if it is only used for Rendering? Could similar functionality be gained using an existing visualization dependency?

Probably not - I am planning to remove the lazy import from beams3d as well before we merge. We can simply replace this with matplotlib as in beams3d.

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.

3 participants