Wire XArraySQLSource into Lumen AI for natural language queries on scientific data#1791
Conversation
Adds a new optional source that enables SQL queries over xarray datasets via xarray-sql/DataFusion. Each data variable is exposed as a SQL table, inheriting from BaseSQLSource to avoid duplication. Includes documentation, 58 tests, and pyproject.toml extras for xarray dependencies.
… metadata - _resolve_chunks: use dataset.chunks directly instead of getattr - get_schema: expand comment explaining TABLESAMPLE/DataFusion chain - normalize_table: add step-by-step comments for clarity - get(): document __limit__ filter usage - _get_table_metadata: replace math.prod with var.size, add auxiliary coord descriptions for non-linear grids (e.g. RASM 2D lat/lon) - Add cfgrib/eccodes deps for GRIB file support - Add tests: engine detection, GRIB I/O, 2D auxiliary coord metadata
- Remove dialect='postgres', inherit 'any' from BaseSQLSource - Add from_dataset() classmethod mirroring DuckDBSource.from_df() - Trim xarray deps: remove cfgrib/eccodes - Simplify normalize_table, _resolve_chunks, get_schema comment - Update dialect test, add from_dataset test (58 pass)
…rib group - Extract _format_aux_coords() helper to reduce indentation in _get_table_metadata - Change .h5/.hdf5 engine map from h5netcdf to netcdf4 (fewer deps, same functionality) - Add xarray-grib optional dep group with cfgrib + eccodes - Update engine detection tests to match
Add xarray file support to Lumen AI so users can query NetCDF, Zarr,
and HDF5 datasets via natural language.
CLI: detect xarray file extensions in _resolve_data, create XArraySQLSource
Upload: register upload handlers for .nc/.h5 drag-and-drop with temp file
cleanup via atexit
Controls: add xarray extensions to TABLE_EXTENSIONS
SQL agent: fix crash when source.tables is a list instead of dict
Source: set dialect=None for sqlglot compat, auto-assign integer coords
for bare dimensions (fixes RASM-type datasets)
Deps: remove h5netcdf, add xarray-grib optional group
8 new tests, 123 total passing. E2E validated with 9 queries + 2 plots
on 11MB climate dataset (NetCDF) and Zarr ocean data.
- Remove .h5/.hdf5/.he5 from XARRAY_ENGINES (xarray doesn't handle pure HDF5 well, h5py is better for those) - Drop h5netcdf from xarray deps in pyproject.toml and pixi.toml - Add separate xarray-grib extra with cfgrib and eccodes - Update engine detection test
e5f921a to
fcb6316
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1791 +/- ##
==========================================
+ Coverage 69.63% 70.02% +0.38%
==========================================
Files 190 193 +3
Lines 31495 32445 +950
==========================================
+ Hits 21933 22721 +788
- Misses 9562 9724 +162 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| import numpy as np | ||
| import xarray as xr |
There was a problem hiding this comment.
Ensure imports are up top (in the future, please do a quick look through your PRs before marking ready for review; it can get a bit tiring to be repeating the same comment).
There was a problem hiding this comment.
Yeah!! i am sorry, i understand will keep this in mind.. Thankyou
Remove top-level import xarray/xarray_sql so installing xarray does not cause it to load by default. Use _check_xarray_available() helper and inline imports in methods that need them. Remove XArraySQLSource from sources/__init__.py -- other optional sources (Snowflake, BigQuery) are not imported there either.
|
Merge conflicts here now. |
Yup due to #1741 merge will fix!! |
Resolve conflicts from holoviz#1741 merge by adopting main's canonical XArraySQLSource implementation (class-level _xarray_engines, classmethod _detect_engine, dialect="any").
| try: | ||
| from ..sources.xarray_sql import XArraySQLSource | ||
| except ImportError: | ||
| return None |
There was a problem hiding this comment.
We should make this a util, rather than try/except Import everywhere.
if not _check_xarray_available():
raise ImportError(
"xarray and xarray-sql are required for XArraySQLSource. "
"Install them with: pip install lumen[xarray]"
)
| """Test that .nc creates separate source while .csv goes to DuckDB.""" | ||
| pytest.importorskip("xarray_sql") | ||
| import numpy as np | ||
| import xarray as xr |
There was a problem hiding this comment.
Please take some time to look at diffs on GitHub and check for imports inlined :'(
There was a problem hiding this comment.
Yess yess for sure.. I will keep this in mind.. this pr is outdated the day you gave me feedback about the imports on top thing!!
| pytest.skip("NetCDF write backend unavailable") | ||
| result = UI._resolve_data(str(nc_path)) | ||
| assert len(result) == 1 | ||
| from lumen.sources.xarray_sql import XArraySQLSource |
There was a problem hiding this comment.
This import is even more insane, not even at the top of the function.
| XARRAY_FILE_EXTENSIONS = ('.nc', '.nc4', '.netcdf', '.h5', '.hdf5', '.he5', '.zarr', '.grib', '.grib2', '.grb', '.grb2') | ||
| XARRAY_UPLOAD_EXTENSIONS = ('nc', 'nc4', 'h5', 'hdf5') | ||
|
|
There was a problem hiding this comment.
Why are constants located between imports?
ahuang11
left a comment
There was a problem hiding this comment.
Take some time to look at GitHub diff! I'm finding myself repeating the same comments and requests.
Yes, yes, I understand. I will keep this in mind. |
| def _check_xarray_available(): | ||
| """Check if xarray and xarray-sql are installed.""" | ||
| try: | ||
| import xarray # noqa | ||
| import xarray_sql # noqa | ||
| return True | ||
| except ImportError: | ||
| return False |
There was a problem hiding this comment.
I suspect more suited to be in util. XArraySQLSource can also use it I think...
| if not _check_xarray_available(): | ||
| return {} | ||
|
|
||
| _temp_files: list[str] = [] | ||
|
|
||
| def _cleanup_temp_files(): | ||
| for path in _temp_files: | ||
| try: | ||
| os.unlink(path) | ||
| except OSError: | ||
| pass | ||
|
|
||
| atexit.register(_cleanup_temp_files) | ||
|
|
||
| def _handle_xarray_upload(context, file_obj, alias, filename): | ||
| from ..sources.xarray_sql import XArraySQLSource | ||
| ext = filename.rsplit('.', 1)[-1] if '.' in filename else 'nc' | ||
| tmp = tempfile.NamedTemporaryFile(delete=False, suffix=f'.{ext}') | ||
| file_obj.seek(0) | ||
| tmp.write(file_obj.read()) | ||
| tmp.flush() | ||
| tmp.close() | ||
| _temp_files.append(tmp.name) | ||
| return XArraySQLSource(uri=tmp.name, name=alias) |
There was a problem hiding this comment.
Not a big fan of nested functions not at the top, e.g.
def func1()
# no other code here please
def nested()
....
other_code
|
Wanted to bring this up again in your description
|
- Move check_xarray_available to lumen/util.py (shared by ui.py and xarray_sql.py) - Remove duplicate ImportError check in _resolve_data (XArraySQLSource handles it) - Move constants after imports, reorder nested functions in upload handler - Move inline import io to top of test file, add skipif decorators
|
Hey Andrew, addressed all your feedback:
Sorry again for the repeated import issues. I'm being more careful reviewing the GitHub diff before pushing now. |
- Resolve controls/base.py -> controls/ingest/ refactor - Define XARRAY_UPLOAD_EXTENSIONS in controls/ingest/constants.py and splat into TABLE_EXTENSIONS (per review) - Derive XARRAY_FILE_EXTENSIONS from XARRAY_UPLOAD_EXTENSIONS so the xarray extension list has a single source of truth - Move XArraySQLSource import to top of lumen/ai/ui.py; drop the inline imports in _get_xarray_upload_handlers and _resolve_data - Reorder _get_xarray_upload_handlers so nested defs come before body - Move MagicMock import to top of tests/ai/test_ui.py
| XARRAY_FILE_EXTENSIONS = tuple(f'.{ext}' for ext in XARRAY_UPLOAD_EXTENSIONS) + ( | ||
| '.netcdf', '.he5', '.zarr', '.grib', '.grib2', '.grb', '.grb2', |
There was a problem hiding this comment.
I'm still confused as to why we have upload extensions vs file extensions. Can you explain?
There was a problem hiding this comment.
You're right, there's no real reason for them to be separate. XArraySQLSource accepts all these formats via a path, and the upload handler just writes to a temp file before calling it, so there's no format supported by one path and not the other.
will fix this!!
The previous split (XARRAY_UPLOAD_EXTENSIONS=4, XARRAY_FILE_EXTENSIONS=11) had no functional reason — XArraySQLSource accepts all 11 formats via URI, and the upload handler writes to a temp file before calling it, so there is no format supported by one path and not the other. - controls/ingest/constants.py: single XARRAY_EXTENSIONS tuple (11 formats, bare names), splatted into TABLE_EXTENSIONS - ui.py: import XARRAY_EXTENSIONS, derive XARRAY_DOTTED_EXTENSIONS for the endswith check in _resolve_data
|
Thanks! This is definitely useful for opening use cases with xarray. In the future, we'll need to handle plotting these gridded datasets better. Really appreciate the work thus far! #1821
|
|
Thanks a lot, Andrew!! Will look into this, and escalate it properly for every type of use cases |
|
Thanks for the quick review and merge!! |
After holoviz#1791 landed, loading a NetCDF file and asking the AI to plot it produces a scatter or line chart rather than a 2D heatmap, because: - hvPlotBaseView had no way to pass a value column (C) to hvplot - The AI agents had no awareness the pipeline was backed by gridded xarray data Changes: - Add `C` param (value column) to hvPlotBaseView; wire into df.hvplot() - Add `quadmesh` to the kind selector - New lumen/ai/gridded.py: detect_gridded() returns coordinate/variable metadata when pipeline.source is XArraySQLSource, else None - BaseViewAgent._generate_yaml_spec: call detect_gridded() once, pass result as `gridded` template variable - BaseViewAgent/main.jinja2 + hvPlotAgent/main.jinja2: conditional block guides LLM to use kind='heatmap' with x/y/C for gridded data - hvPlotAgent._extract_spec: drop by/groupby when kind=heatmap Addresses holoviz#1821 (1/3)










I've been building a climate data analysis tool with xarray datasets and wanted to use Lumen AI to query them with natural language. Since XArraySQLSource landed in #1741, this wires it into the AI layer so lumen-ai serve data.nc just works, including drag-and-drop upload of .nc and .h5 files.
Depends on: #1741 (XArraySQLSource)
E2E validated with 9 queries + 2 plots on an 11MB climate dataset (screenshots in comments).