Skip to content

Guard optional data-module imports against missing deps (#179) - #230

Open
munzzyy wants to merge 1 commit into
openclimatefix:mainfrom
munzzyy:fix/optional-data-imports
Open

munzzyy wants to merge 1 commit into
openclimatefix:mainfrom
munzzyy:fix/optional-data-imports

Conversation

@munzzyy

@munzzyy munzzyy commented Jul 5, 2026

Copy link
Copy Markdown

Follow-up to #179.

The nnjai_wrapp import from the original report is gone, and graph_weather/__init__.py already guards from .data.nnja_ai import SensorDataset in a try/except. But graph_weather/data/__init__.py still imports AnemoiDataset, SensorDataset, and WeatherStationReader unconditionally, and those pull in pandas, xarray, anemoi.datasets, and nnja-ai at import time. None of those are declared dependencies of graph_weather, so a plain pip install graph_weather still crashes on bare import graph_weather - just with a different missing-module error depending on what's installed (pandas, then xarray, then an nnja-ai ImportError).

This wraps each of the three imports in data/__init__.py in the same try/except pattern already used for SensorDataset at the package root, falling back to None when the optional dep isn't installed. Also applied the same guard to WeatherStationReader in the root __init__.py, since it had the same unconditional-import problem.

Added a test that blocks pandas/xarray/anemoi.datasets/nnja_ai at import time and checks graph_weather.data still imports with all three classes falling back to None.

Verified locally in a clean venv with pip install --no-deps -e . and none of the optional deps installed - import graph_weather (well, graph_weather.data specifically, since I couldn't get a working torch build in this environment) crashed on pandas before this change, imports clean after. Couldn't run the full test suite here since I hit the Windows long-path issue installing torch, so I'd appreciate someone running CI on this one rather than trusting my local run alone.

graph_weather/data/__init__.py unconditionally imported AnemoiDataset,
SensorDataset, and WeatherStationReader, each of which pulls in pandas,
xarray, anemoi.datasets, or nnja-ai at import time. None of those are
declared dependencies of graph_weather, so a plain pip install still
crashes on bare import graph_weather even after the nnja_ai guard added
to the top-level __init__.py for openclimatefix#179.

Wrap each optional submodule import in the same try/except pattern
already used at the package root, falling back to None like
SensorDataset already does. Add a test that blocks the optional deps
and checks graph_weather.data still imports.

Follow-up to openclimatefix#179.
@munzzyy

munzzyy commented Jul 5, 2026

Copy link
Copy Markdown
Author

Dug into the failing checks here.

pytest (macos + ubuntu): both jobs crash during test collection, not during any actual test run. The traceback shows a native-library abort in torch_scatter's extension loader, called from torch_geometric/typing.py:139, triggered while pytest is importing tests/test_fgn.py (macOS: Fatal Python error: Aborted / std::length_error: vector from libc++abi; Ubuntu: Fatal Python error: Segmentation fault). Neither of those files is touched by this PR. I checked the "Python package" workflow history on main and it's been failing the same way since at least June 4 - this predates this branch by a month, so it's an existing environment/pixi-lockfile problem (likely a torch/torch_geometric/torch_scatter version mismatch), not something this diff introduced.

pre-commit.ci: the failing hook is ruff, and the violations it lists are all in graph_weather/models/aurora/, graph_weather/models/cafa/, and graph_weather/data/anemoi_dataloader.py - none of which this PR touches. I pulled main clean and ran ruff==0.15.15 (the version pinned in .pre-commit-config.yaml) against it directly: same 156 errors, same files. I also ran ruff against this branch in full and got the identical count, so this diff adds zero new lint violations - it's pre-existing debt across the repo.

One more thing while I was in here: tests/test_optional_imports.py blocks pandas/xarray/anemoi.datasets/nnja_ai and checks graph_weather.data still imports, but that import first runs the parent graph_weather/__init__.py, which does an unguarded from .models.analysis import GraphWeatherAssimilator (line 11) - pulling in torch_geometric transitively through models/__init__.py -> fengwu_ghr/layers.py. In the pixi CI env torch_geometric is always present so this doesn't surface as a failure there, but it means the test's real guarantee is narrower than the docstring claims for a genuinely bare install. Since torch_geometric is actually a hard runtime dependency of the core models (used directly in layers/encoder.py, layers/assimilator_encoder.py, etc.) and isn't in [project.dependencies] at all, guarding it with try/except the same way as the dataloaders would be wrong - the real fix there is adding it to dependencies in pyproject.toml, which is a packaging change outside what this PR set out to do. Flagging it rather than folding it in here.

Net: the two failing checks aren't caused by this branch. Happy to rebase once the segfault/lockfile issue on main gets sorted.

@munzzyy

munzzyy commented Jul 5, 2026

Copy link
Copy Markdown
Author

Filed #232 for the segfault so it's tracked separately from this PR.

@munzzyy

munzzyy commented Jul 6, 2026

Copy link
Copy Markdown
Author

Dug into this failure before pushing anything else. Both jobs fail before any test in this diff runs — pytest crashes during collection with a native segfault:

Fatal Python error: Segmentation fault
  File ".../torch_scatter/__init__.py", line 16 in <module>
  File ".../torch/_ops.py", line 1442 in load_library

on macOS (exit 134, libc++abi ... std::length_error: vector) and Ubuntu (exit 139). That crash happens while importing graph_weather itself, not from anything this PR touches (graph_weather/__init__.py, graph_weather/data/__init__.py, tests/test_optional_imports.py).

Checked main directly to rule out our diff: same segfault, same torch_scatter/__init__.py:16 frame, on both runners, at the latest push to main (run https://github.com/openclimatefix/graph_weather/actions/runs/28750434951). This is a pinned dependency ABI break in the pixi CPU env (torch 2.7.0 + torch_scatter 2.1.2 / pyg_lib), not something this PR introduced.

Same story for pre-commit.ci: running pre-commit run --all-files locally on main gives 156 pre-existing ruff docstring/line-length errors in files this PR never touches (aurora/, cafa/, weathermesh/, fengwu_ghr/, train/, setup.py). This branch actually comes in a bit cleaner (147) since the files it edits pass ruff clean.

So both checks are red on main already and this diff doesn't add to either failure. Flagging so it doesn't block review — happy to rebase once the pytest/torch_scatter issue gets fixed upstream.

@munzzyy

munzzyy commented Jul 6, 2026

Copy link
Copy Markdown
Author

Same situation as #231: the pytest segfault is the main-branch crash tracked in #232, and the pre-commit ruff failures are in the new aurora/cafa modules, not in anything changed here (checked with ruff 0.15.15 and the repo config). Will rebase when main is green.

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.

1 participant