Skip to content

Add experimental XarraySource#1751

Closed
SanketMeghale wants to merge 3 commits into
holoviz:mainfrom
SanketMeghale:feature/xarray-source
Closed

Add experimental XarraySource#1751
SanketMeghale wants to merge 3 commits into
holoviz:mainfrom
SanketMeghale:feature/xarray-source

Conversation

@SanketMeghale

@SanketMeghale SanketMeghale commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

Summary

This draft PR adds an experimental native XarraySource for xarray-backed datasets.

Current MVP scope:

  • add lumen.sources.xarray.XarraySource
  • expose Dataset.data_vars as logical tables
  • support coordinate-aware filtering for 1D coordinates
  • flatten query results to pandas DataFrames for downstream compatibility
  • add docs and example config/demo for NetCDF/Zarr-backed datasets
  • add focused test coverage for schema, filtering, metadata, URI loading, and example rendering

This is intended for early design feedback rather than final review.

later improvements

  • aggregation in the source API
  • SQL interoperability beyond DataFrame conversion
  • multi-file dataset support

Validation

  • python -m pytest lumen/tests/sources/test_xarray.py -q

Refs #1748

@codecov

codecov Bot commented Mar 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.78937% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.94%. Comparing base (4b4ccb2) to head (3e3a312).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lumen/sources/xarray.py 89.58% 30 Missing ⚠️
lumen/tests/sources/test_xarray.py 97.81% 5 Missing ⚠️
lumen/sources/__init__.py 70.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1751      +/-   ##
==========================================
+ Coverage   67.43%   67.94%   +0.51%     
==========================================
  Files         168      170       +2     
  Lines       27947    28498     +551     
==========================================
+ Hits        18845    19364     +519     
- Misses       9102     9134      +32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ahuang11 ahuang11 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think to make this work with Lumen (AI part), we'll probably need to base it off BaseSQLSource and https://github.com/alxmrs/xarray-sql, so that AI can write the transforms.

Separately, I think there's a lot of AI things that are undesired, e.g.

    This first draft is intentionally conservative:
    - accepts an in-memory xarray.Dataset
    - exposes Dataset.data_vars as logical tables
    - supports coordinate-based filtering via .sel(...)
    - returns pandas DataFrames for downstream compatibility

This should be part of your PR description, not docstring.

@SanketMeghale

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed feedback!

That makes sense, aligning this with BaseSQLSource and xarray-sql would definitely make it more flexible and allow AI-generated transforms in a cleaner way. I’ll explore restructuring the implementation in that direction.

Also noted on keeping the AI behavior minimal and predictable , I’ll avoid adding unnecessary complexity there.

And thanks for pointing out the docstring vs PR description , I’ll move that explanation to the PR description and keep the docstrings focused on code-level details.

Appreciate the guidance!

@ahuang11

ahuang11 commented May 6, 2026

Copy link
Copy Markdown
Contributor

Superseded by #1741

@ahuang11 ahuang11 closed this May 6, 2026
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.

2 participants