FixDataQueryDeps — Overhaul dependency model, env setup tooling, CI, and release workflow#266
FixDataQueryDeps — Overhaul dependency model, env setup tooling, CI, and release workflow#266
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to fix broken production installs (Poetry install without dev dependencies) caused by DataQuery being imported at package import-time, and adds CI checks to validate the library in both “prod-only” and “with dev” dependency modes.
Changes:
- Make the Poetry
devdependency group explicitly optional. - Change
aodn_cloud_optimisedpackage initialization to tolerate missing dev-only dependencies by guardingDataQuery. - Update CI build workflow to verify imports/behavior in both production-only and dev modes.
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Marks Poetry dev group as optional to support prod-only installs. |
poetry.lock |
Updates lock metadata hash after dependency/group changes. |
notebooks/requirements.txt |
Refreshes exported notebook requirements (new deps/markers). |
aodn_cloud_optimised/bin/validate_data_query.py |
Adds shebang and removes stray blank line in version extraction loop. |
aodn_cloud_optimised/__init__.py |
Wraps DataQuery import with a fallback stub to allow prod installs without dev deps. |
.github/workflows/build.yml |
Adds prod/dev-mode verification steps after building. |
| try: | ||
| # Attempt to load the dev-only module | ||
| from .lib import DataQuery | ||
|
|
||
| __all__.append("DataQuery") | ||
| except (ImportError, ModuleNotFoundError): | ||
| # Define a guard function that only raises an error if called |
There was a problem hiding this comment.
The try/except around from .lib import DataQuery catches any ImportError, which can silently mask real bugs inside aodn_cloud_optimised.lib.DataQuery (or its imports) even when dev deps are installed, and then replace it with the placeholder. Consider narrowing the handling (e.g., only fallback for missing optional deps / specific ModuleNotFoundError cases) and/or preserving the original exception as the cause so failures are diagnosable.
| # - name: Verify build | ||
| # run: | | ||
| # pip install dist/*.whl --no-deps | ||
|
|
||
| - name: Verify build (Production Mode) | ||
| run: | | ||
| # 1. Install ONLY production dependencies (no dev tools) | ||
| # --sync ensures the environment matches the lock file exactly | ||
| poetry install --without dev --sync | ||
|
|
There was a problem hiding this comment.
The new verification steps no longer install/test the built wheel artifact (the previous pip install dist/*.whl --no-deps check is commented out). As written, this validates poetry install from the repo source, not the package produced by poetry build, so packaging issues in the wheel/sdist could slip through. Consider restoring a wheel install test in an isolated env for both production and dev scenarios.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sion as source of truth - Add .poetry-version file (2.3.3) with full sync checklist for all locations - GHA workflows (build, pre-commit, release) now read version from .poetry-version via 'Read Poetry version' step — no manual edits needed in workflows on bump - Upgrade poetry 2.2.1 → 2.3.3 across all sync points: environment.yml, pyproject.toml dev extras, .pre-commit-config.yaml rev: - Fix stale mismatch: pre-commit.yml had poetry 2.3.3 in snok, now reads from file - Pin pre-commit 4.5.1 exactly (was >=4.0.1) in pyproject.toml - Regenerate poetry.lock with poetry 2.3.3 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e_venvs.sh with updated docs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #266 +/- ##
=======================================
Coverage ? 65.89%
=======================================
Files ? 28
Lines ? 5324
Branches ? 0
=======================================
Hits ? 3508
Misses ? 1816
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Describes the workflow_dispatch-based release.yml: - how to trigger patch/minor/major bumps from GitHub Actions UI - what each workflow step does (test, bump, frozen/unfrozen wheels, tag, release) - frozen vs unfrozen wheel explanation and install instructions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… → --extras - Remove hardcoded ENV POETRY_VERSION=2.2.1; COPY .poetry-version and read it via shell substitution in the RUN step — Dockerfile now auto-syncs on bump - Fix broken 'poetry install --with dev' (old group syntax, removed in Phase 2) → 'poetry install --extras "notebooks tests docs dev"' - Update .poetry-version comment to list Dockerfile as auto-read (no manual sync) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…miniforge_venvs.sh Replace stale mamba env create + AodnCloudOptimisedQuery instructions with: - Option 1: make notebooks (Poetry venv) - Option 2: ./setup_miniforge_venvs.sh notebooks (conda/mamba → AodnCloudOptimised_notebooks) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ile when running make within a poetry venv
|
@copilot create a thorough summary of what was changed in this PR, many new features |
Here is a thorough summary of all changes in this PR: PR Summary —
|
| Extra | Contents | When to use |
|---|---|---|
notebooks |
DataQuery + viz libs (cartopy, matplotlib, seaborn, folium, tqdm, papermill, …) |
Jupyter notebooks, DataQuery API |
tests |
pytest, coverage, moto | Running the test suite |
docs |
Sphinx, sphinx-rtd-theme, asciinema | Building documentation |
dev |
poetry, pre-commit, ipdb, virtualenv | Contributor tooling |
python-levenshteinandfuzzywuzzy(DataQuery-only deps) moved from core →notebooksextra.- Python version constraint tightened to
>=3.11,<3.13(avoids source builds for h5py/netCDF4). poetry-corebumped to2.3.2.
🆕 New File: .poetry-version
A single source of truth for the Poetry version used across all CI workflows, Dockerfile, and environment.yml. All workflows now read from this file instead of hardcoding the version.
🆕 New File: Makefile
Convenience targets for environment management:
make core # core data-processing only
make notebooks # core + DataQuery/Jupyter
make tests # core + tests + notebooks (CI)
make docs # core + Sphinx
make dev # everything — full contributor setup
make clean # wipe .venv and reinstall
Each target calls poetry sync --extras "..." and correctly finds the global poetry binary even when run from inside an active venv.
🆕 New File: setup_miniforge_venvs.sh
A bash script for contributors who prefer named conda/mamba environments over Poetry's single .venv. Creates one named environment per install mode (e.g. AodnCloudOptimised_dev, AodnCloudOptimised_notebooks).
./setup_miniforge_venvs.sh dev # full contributor setup
./setup_miniforge_venvs.sh notebooks # notebooks only
./setup_miniforge_venvs.sh all # create all environments🆕 New File: poetry.toml
Ensures poetry always creates the .venv inside the project root (virtualenvs.in-project = true), so IDE tooling and make targets always find the right environment.
⚙️ CI Workflows
build.yml (updated):
- Reads Poetry version from
.poetry-version. - Removed unreliable venv caching.
- Uses
make tests(=poetry sync --extras "tests notebooks") for dependency install. - Added 3-tier wheel verification: Production Mode (core only), Notebooks Mode, and Dev Mode, each installing the built wheel and validating real imports.
pre-commit.yml (updated):
- Reads Poetry version from
.poetry-version. - Uses
make devfor full contributor install instead of barepoetry install.
release.yml (major overhaul):
- Changed trigger from
on: release: created→on: workflow_dispatchwith abump_typechoice (patch/minor/major). No manual tag creation needed anymore. - Fully automated pipeline: tests → version bump (Python script, no Poetry CLI needed) → build (frozen + unfrozen wheel + sdist) → verify → git commit + tag + push → GitHub Release with all artifacts.
- Produces two wheels in
dist-artifacts/: a frozen wheel (all deps pinned viapoetry-plugin-freeze) and an unfrozen wheel (flexible ranges). - Steps guarded with
if: ${{ !env.ACT }}so the full pipeline can be tested locally withactwithout pushing or creating releases.
📄 Documentation
docs/development/release.rst (new file):
Full documentation for the new automated release process, covering the workflow trigger, each step, the two wheel types, local testing with act, and manual version bump instructions.
docs/development/installation.rst (major rewrite):
- New Dependency Extras table.
- New Makefile (recommended) section.
setup_miniforge_venvs.shsection replacing old conda/mamba instructions.- Core-only install section.
- Updated
poetry exportcommands for the new extras model.
README.md (updated):
- New Core install section with
make core. - Updated Development section pointing to
make dev/setup_miniforge_venvs.sh dev.
notebooks/README.md (updated):
- Two local setup options: Poetry (
make notebooks) and Mamba/Conda (setup_miniforge_venvs.sh notebooks).
🔧 Other Changes
environment.yml: Updated to Poetry 2.3.3; removed-r requirements.txt(handled by extras now).Dockerfile: Reads Poetry version from.poetry-versionat build time.requirements.txt/notebooks/requirements.txt: Regenerated after pyproject.toml restructure.poetry.lock: Regenerated with Poetry 2.3.x to match updatedpyproject.toml..gitignore: Addednotebooks/Untitled.ipynbandartifacts/.aodn_cloud_optimised/bin/validate_data_query.py: Added shebang, minor cleanup.
|
Looks like maybe
|
Fixed by running |
What changed
Bug Fix
aodn_cloud_optimised/__init__.py: Removed the top-levelDataQueryimport entirely.DataQuerymust now be imported directly (from aodn_cloud_optimised.lib.DataQuery import GetAodn), which fixespoetry install(without extras) being broken at import time due to missing optional dependencies.Dependency Architecture (pyproject.toml)
[tool.poetry.group.dev.dependencies]to PEP 621[project.optional-dependencies]with four named extras:notebooks— DataQuery + visualisation libs (cartopy, matplotlib, seaborn, folium, tqdm, papermill, …)tests— pytest, coverage, motodocs— Sphinx and related toolsdev— contributor tooling (poetry, pre-commit, ipdb, virtualenv)python-levenshtein/fuzzywuzzy(DataQuery-only) moved from core →notebooksextra.>=3.11,<3.13.poetry-corebumped to2.3.2.New:
.poetry-versionSingle source of truth for the Poetry version used across all CI workflows, Dockerfile, and
environment.yml.New:
MakefileConvenience targets for environment management:
make core/make notebooks/make tests/make docs/make dev/make cleanEach target calls
poetry sync --extras "..."and correctly locates the globalpoetrybinary even when invoked from inside an active venv.New:
setup_miniforge_venvs.shBash script to create named conda/mamba environments (one per install mode:
core,notebooks,tests,docs,dev,all). Alternative to the Makefile for contributors who prefer named conda environments.New:
poetry.tomlEnsures Poetry always creates
.venvinside the project root (virtualenvs.in-project = true).CI Workflows
build.yml: Reads Poetry version from.poetry-version; removed unreliable venv caching; usesmake testsfor install; added 3-tier wheel verification (Production / Notebooks / Dev modes) each installing the built wheel and validating real imports.pre-commit.yml: Reads Poetry version from.poetry-version; usesmake devinstead of barepoetry install.release.yml(major overhaul):on: release: created→on: workflow_dispatchwith abump_typechoice (patch/minor/major).if: ${{ !env.ACT }}guards allow end-to-end local testing withactwithout pushing or creating releases.Documentation
docs/development/release.rst(new): Full documentation for the automated release workflow, wheel types, and local testing withact.docs/development/installation.rst(rewritten): Covers PEP 621 extras table, Makefile workflow,setup_miniforge_venvs.sh, and core-only install.README.md: New Core install section, updated Development section.notebooks/README.md: Two local setup options — Poetry (make notebooks) and Mamba/Conda (setup_miniforge_venvs.sh notebooks).Other
environment.yml,Dockerfile: Updated to use.poetry-version.requirements.txt/notebooks/requirements.txt: Regenerated after pyproject.toml restructure.poetry.lock: Regenerated with Poetry 2.3.x..gitignore: Addednotebooks/Untitled.ipynbandartifacts/.aodn_cloud_optimised/bin/validate_data_query.py: Added shebang, minor cleanup.