Skip to content

Conversation

@coreyjadams
Copy link
Collaborator

@coreyjadams coreyjadams commented Feb 10, 2026

@coreyjadams coreyjadams marked this pull request as ready for review February 10, 2026 19:57
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 10, 2026

Greptile Overview

Greptile Summary

Migrated all datapipes module imports to use OptionalImport pattern for better dependency management.

Key changes:

  • Removed datapipes exclusion from .importlinter contract - datapipes now properly checks optional imports
  • Replaced hardcoded try/except blocks and ImportError raises with lazy OptionalImport from physicsnemo.core.version_check
  • Added TYPE_CHECKING guards where needed for type annotations (e.g., PyGData, netcdf_file)
  • Updated all type hints to use string quotes for forward references (e.g., "dali.Pipeline", "xr.Dataset")
  • Removed dask.diagnostics.ProgressBar usage in healpix module (simplified to direct compute call)

Packages migrated to OptionalImport:

  • nvidia.dali and nvidia.dali.plugin.pytorch
  • netCDF4 and scipy.io
  • torch_geometric, torch_geometric.data, torch_geometric.loader
  • scipy.sparse and scipy.spatial
  • sparse_dot_mkl
  • tfrecord.torch.dataset
  • vtk
  • xarray

The changes ensure consistent error messages with install hints across the codebase and defer imports until actual usage.

Important Files Changed

Filename Overview
.importlinter Removed datapipes exclusion from import linting - datapipes now uses OptionalImport for optional dependencies
physicsnemo/datapipes/climate/climate.py Converted DALI, netCDF4, and scipy imports to OptionalImport pattern with proper TYPE_CHECKING guards
physicsnemo/datapipes/gnn/bsms.py Converted scipy.sparse and sparse_dot_mkl imports to OptionalImport pattern, removed try/except with warning
physicsnemo/datapipes/healpix/data_modules.py Converted xarray import to OptionalImport pattern, removed dask.diagnostics.ProgressBar usage

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Raise ImportError instead of RuntimeError - it just is deferred to runtime.
Clean up TE usage in MLP.
Fix the way OptionalImport was handling dunders.
@coreyjadams coreyjadams changed the base branch from optional_deps-2 to main February 10, 2026 23:12
@coreyjadams
Copy link
Collaborator Author

/blossom-ci

@mnabian mnabian self-requested a review February 10, 2026 23:37
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