Skip to content

Enable Dask by default for chunked indicators - #170

Merged
baptistehamon merged 4 commits into
baptistehamon:mainfrom
akritaag:review/lsapy-follow-up
Sep 12, 2026
Merged

baptistehamon merged 4 commits into
baptistehamon:mainfrom
akritaag:review/lsapy-follow-up

Conversation

@akritaag

@akritaag akritaag commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

This fixes the Dask issue from my review, following our discussion in #164.

With chunked input, lsa.run() currently raises an error unless Dask options are passed explicitly. The two-line fix defaults chunked indicators to dask="parallelized", while respecting any setting the caller provides. Dask infers the output dtype unless one is supplied, as described in xarray’s API.

The supporting changes add a lsapy[dask] install option, a chunked example, docstrings, a changelog entry, and tests for lazy execution, numerical agreement, and explicit overrides. The example also runs in a fresh Python process.

Validation

Tested on Python 3.12.14, macOS arm64, with xarray 2026.7.0, Dask 2026.8.0 and NumPy 2.5.3.

  • Full suite: 100 passed, including 11 Dask and example cases. The existing 7,507 warnings remain. The test runner redirected Pooch’s cache to a writable local directory.
  • Applicable pre-commit hooks passed for the changed files.
  • The chunked documentation example runs successfully.
  • Strict Sphinx HTML build reports four existing unknown-link-target errors for xarray, Dask and matplotlib in docs/index.rst, which is unchanged from the base branch. Existing notebooks were rendered without execution.

No performance benchmark or cross-platform test matrix was run locally.

Pull request checklist

  • Addresses an existing issue: LSAPy Review #2 #164 (Dask item)
  • Tests added
  • Documentation updated
  • CHANGELOG.rst updated

AI assistance

OpenAI Codex was used in an agentic workflow to prepare the two-line runtime change. The supporting changes are mostly tests and documentation, plus contributor credit in AUTHORS.rst and .zenodo.json. Automated checks do not establish human review or pyOpenSci acceptance.

@akritaag akritaag changed the title Support chunked suitability computation and document review follow-ups Enable Dask by default for chunked indicators Sep 8, 2026
Comment thread docs/notebooks/chunked.rst Outdated
Comment thread docs/notebooks/chunked.rst Outdated
Comment thread docs/notebooks/chunked.rst Outdated
Comment thread docs/notebooks/chunked.rst Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask.py Outdated
Comment thread tests/test_dask_example.py

@baptistehamon baptistehamon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks a lot @akritaag! I just suggested minor changes mainly for consistency with other files. Feel free to also update the the AUTHORS.rst and .zenodo.json if you want!

@baptistehamon baptistehamon added docs Improvements or additions to documentation enhancement New feature or request labels Sep 9, 2026
@akritaag

Copy link
Copy Markdown
Contributor Author

Thanks @baptistehamon! Applied the naming and wording suggestions in e4e77ec, and added my name under Contributors in AUTHORS.rst and .zenodo.json. Thanks for suggesting it!

I made a small adjustment to the override assertion and analysis assertion: both compare against exp. That preserves the comparison with the unchunked result; res also isn't defined in the analysis test.

On the example test, the new example is an .rst file, so nbval doesn't collect it. Running pytest --nbval --collect-only -q docs/notebooks/ collected 12 cells from the existing notebooks, but not this example. I've kept the test and added a docstring explaining why. The nbval documentation describes its notebook collection.

All 11 Dask/example tests and Ruff checks passed locally. Read the Docs and pre-commit.ci are now green too.

@baptistehamon

Copy link
Copy Markdown
Owner

LGTM ! Thanks again @akritaag !

@baptistehamon
baptistehamon merged commit 8ba3097 into baptistehamon:main Sep 12, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants