TDD demo: xorq↔polars isolation test (red→green)#704
Conversation
📦 TestPyPI package publishedpip install --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo==0.13.5.dev25429745295or with uv: uv pip install --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo==0.13.5.dev25429745295MCP server for Claude Codeclaude mcp add buckaroo-table -- uvx --from "buckaroo[mcp]==0.13.5.dev25429745295" --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo-table📖 Docs preview🎨 Storybook preview |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7010679ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import pandas as pd | ||
|
|
||
| from .buckaroo_widget import BuckarooWidget | ||
| from .customizations.pl_autocleaning_conf import NoCleaningConfPl |
There was a problem hiding this comment.
In a buckaroo[xorq] install that does not include the polars extra, importing buckaroo.xorq_buckaroo still imports buckaroo.customizations.pl_autocleaning_conf, which in turn imports the polars command stack. That makes the new xorq widget unusable unless polars is also installed, and it is exactly the dependency leak the added isolation test blocks via buckaroo.customizations.pl_autocleaning_conf. Define/use an xorq-local no-cleaning config instead of reusing NoCleaningConfPl.
Useful? React with 👍 / 👎.
Adds tests/unit/test_xorq_polars_isolation.py — a subprocess + meta-path-blocker check that buckaroo.xorq_buckaroo imports without polars and vice versa. Brings in the initial XorqBuckarooWidget on top, which reuses NoCleaningConfPl from polars-land — so the test should fail on this commit and pass on the next one. This is the "test must be seen failing on CI" half of the TDD pair; the fix lands in the follow-up commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
paddymul
force-pushed
the
tdd/xorq-polars-isolation-demo
branch
from
a701067 to
a5feb3f
Compare
Replace the borrowed NoCleaningConfPl with a local NoCleaningConfXorq. Importing NoCleaningConfPl from buckaroo.customizations.pl_autocleaning_conf transitively imported polars_commands -> polars, which broke a polars-less ``buckaroo[xorq]`` install. NoCleaningConfXorq is just an empty AutocleaningConfig — no commands registered, since the xorq path doesn't run the lisp interpreter on ibis exprs anyway. Pairs with the previous commit. The isolation test was seen failing on CI on the previous commit; on this commit it passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Purpose
Demonstration PR for the TDD discipline in CLAUDE.md — the test must be seen failing on CI before the fix lands. Two commits, intended to be viewed in order:
RED —
a5feb3ffaddstests/unit/test_xorq_polars_isolation.pyplus the originalXorqBuckarooWidgetfrom PR feat(xorq): XorqBuckarooWidget — buckaroo against an ibis/xorq expression #703's first commit, which still reusesNoCleaningConfPlfrom polars-land. The isolation test (test_xorq_buckaroo_imports_without_polars) fails: importingbuckaroo.xorq_buckarooin a subprocess with polars-side modules blocked surfaces anImportError: BLOCKED: buckaroo.customizations.pl_autocleaning_conf.GREEN —
e30136b2introduces a self-containedNoCleaningConfXorqinxorq_buckaroo.pyand switchesautoclean_confto it. The cross-import is gone; the isolation test passes.What to look at
pytest.importorskip("xorq.api")short-circuits.This PR is not for merge — it's a demo so we can read CI output for both states. PR #703 already carries the same fix and the same regression test.
🤖 Generated with Claude Code