Skip to content

Removed jafgen from pyproject.toml#28

Merged
AivanF merged 4 commits intomainfrom
aivan/2026-04-13-jafgen-dep
Apr 14, 2026
Merged

Removed jafgen from pyproject.toml#28
AivanF merged 4 commits intomainfrom
aivan/2026-04-13-jafgen-dep

Conversation

@AivanF
Copy link
Copy Markdown
Collaborator

@AivanF AivanF commented Apr 13, 2026

PyPI rejects projects with dependencies outside of PyPI, and we need that specific commit (not merged still) that fixes a bug.

Summary by CodeRabbit

  • Chores

    • CI now installs dev dependency groups, adds a step to install jaffle-shop-generator (pinned commit) for notebook runs, and excludes docs from linting hooks.
    • Removed jafgen from declared project dependencies and extras; it should be installed via the notebooks or manually.
  • Documentation

    • Example notebooks now include explicit install/setup cells, clarified prerequisites, refreshed execution outputs/metadata, and minor formatting cleanups.

@AivanF AivanF requested a review from ZmeiGorynych April 13, 2026 12:44
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ca7c588b-4f69-466b-98a0-245f122c6db5

📥 Commits

Reviewing files that changed from the base of the PR and between 099bebf and f637914.

📒 Files selected for processing (7)
  • docs/examples/02_sql_vs_dsl/sql_vs_dsl_nb.ipynb
  • docs/examples/03_auto_ingest/auto_ingest_nb.ipynb
  • docs/examples/04_time/time_nb.ipynb
  • docs/examples/05_joined_measures/joined_measures_nb.ipynb
  • docs/examples/05_joins/joins_nb.ipynb
  • docs/examples/06_multistage_queries/multistage_queries_nb.ipynb
  • docs/examples/07_aggregations/aggregations_nb.ipynb
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/examples/05_joins/joins_nb.ipynb
  • docs/examples/07_aggregations/aggregations_nb.ipynb

📝 Walkthrough

Walkthrough

Removed the jafgen (jaffle-shop-generator) dependency from pyproject.toml and moved its installation into CI and multiple example notebooks via a pinned pip install of the Git commit; CI was adjusted to include dev deps and run the new install step; many notebooks had install cells and JSON/source formatting changes.

Changes

Cohort / File(s) Summary
CI & project config
​.github/workflows/ci.yml, pyproject.toml
Removed jafgen from pyproject.toml and extras; CI command changed to poetry install -E all --with dev and a poetry run pip install ... step was added to install jaffle-shop-generator from a pinned git commit before lint/tests.
Pre-commit
.pre-commit-config.yaml
Added exclude: ^docs/ to Ruff hooks so files under docs/ are skipped by those hooks.
Example notebooks
docs/examples/02_sql_vs_dsl/sql_vs_dsl_nb.ipynb, docs/examples/03_auto_ingest/auto_ingest_nb.ipynb, docs/examples/04_time/time_nb.ipynb, docs/examples/05_joined_measures/joined_measures_nb.ipynb, docs/examples/05_joins/joins_nb.ipynb, docs/examples/06_multistage_queries/multistage_queries_nb.ipynb, docs/examples/07_aggregations/aggregations_nb.ipynb
Inserted an install-deps cell that pip install -q a pinned jaffle-shop-generator git commit; updated prerequisites text to reference the new cell; normalized notebook JSON (trailing newline) and converted many source fields to multi-line arrays; updated execution counts and captured outputs; minor refactoring of engine.execute(query=...) formatting and some printed field names.
Docs metadata/formatting only
docs/examples/* (general)
Mostly notebook metadata updates (Python version), re-execution outputs, and formatting-only JSON changes across example notebooks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • ZmeiGorynych

Poem

🐰 I hopped through cells and lines tonight,
Pulled jafgen out of pyproject's sight;
CI now fetches a pinned commit in place,
Notebooks wake with a quiet install embrace,
Nibbles of data, tidy and light. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change in the PR: removing jafgen from pyproject.toml, which is confirmed in the detailed file changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch aivan/2026-04-13-jafgen-dep

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/examples/02_sql_vs_dsl/sql_vs_dsl_nb.ipynb`:
- Around line 19-20: The Markdown string in the notebook cell currently has an
extra trailing backtick: "\"**Prerequisites:** `pip install
motley-slayer[examples]` (jafgen is installed by the cell below)`\""; edit that
string (the notebook cell content containing the Prerequisites line) to remove
the final backtick so it reads "**Prerequisites:** `pip install
motley-slayer[examples]` (jafgen is installed by the cell below)".

In `@docs/examples/03_auto_ingest/auto_ingest_nb.ipynb`:
- Line 17: The Markdown string currently contains a stray trailing backtick:
"**Prerequisites:** `pip install motley-slayer[examples]` (jafgen is installed
by the cell below)`"; edit that cell text to remove the final backtick so it
reads "**Prerequisites:** `pip install motley-slayer[examples]` (jafgen is
installed by the cell below)" and save the notebook.

In `@docs/examples/04_time/time_nb.ipynb`:
- Line 16: Remove the stray trailing backtick from the Markdown string in the
notebook cell (the line containing "**Prerequisites:** `pip install
motley-slayer[examples]` (jafgen is installed by the cell below)`"); update that
string to end without the extra backtick so it reads correctly as Markdown.

In `@docs/examples/05_joined_measures/joined_measures_nb.ipynb`:
- Line 18: Fix the malformed inline-code in the notebook cell that contains the
string "**Prerequisites:** `pip install motley-slayer[examples]` (jafgen is
installed by the cell below)`" by removing the extra trailing backtick so the
backticks correctly wrap the pip command; update that cell’s markdown text to
"**Prerequisites:** `pip install motley-slayer[examples]` (jafgen is installed
by the cell below)".

In `@docs/examples/05_joins/joins_nb.ipynb`:
- Line 20: Fix the malformed Markdown in the prerequisites string by removing
the extra trailing backtick: change the cell content line that currently reads
"**Prerequisites:** `pip install motley-slayer[examples]` (jafgen is installed
by the cell below)`" to properly close backticks only around the pip command
(i.e., no trailing backtick after the parenthesis) so the text around the symbol
remains valid Markdown.

In `@docs/examples/06_multistage_queries/multistage_queries_nb.ipynb`:
- Line 16: Edit the notebook cell containing the prerequisites string
"**Prerequisites:** `pip install motley-slayer[examples]` (jafgen is installed
by the cell below)`" and remove the stray trailing backtick at the end so the
Markdown becomes well-formed (i.e., end the line with "...below)" instead of
"...below)`"). Ensure the updated cell preserves the inline code backticks
around pip install only.

In `@docs/examples/07_aggregations/aggregations_nb.ipynb`:
- Around line 14-15: The Markdown in the notebook's prerequisites string has an
extra trailing backtick that breaks inline-code formatting; edit the string in
the cell containing "**Prerequisites:** `pip install motley-slayer[examples]`
(jafgen is installed by the cell below)`" to remove the stray final backtick so
it becomes "**Prerequisites:** `pip install motley-slayer[examples]` (jafgen is
installed by the cell below)".
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2972399e-2002-43e8-977a-4a1dde799eb5

📥 Commits

Reviewing files that changed from the base of the PR and between a718acc and 1765a04.

📒 Files selected for processing (9)
  • .github/workflows/ci.yml
  • docs/examples/02_sql_vs_dsl/sql_vs_dsl_nb.ipynb
  • docs/examples/03_auto_ingest/auto_ingest_nb.ipynb
  • docs/examples/04_time/time_nb.ipynb
  • docs/examples/05_joined_measures/joined_measures_nb.ipynb
  • docs/examples/05_joins/joins_nb.ipynb
  • docs/examples/06_multistage_queries/multistage_queries_nb.ipynb
  • docs/examples/07_aggregations/aggregations_nb.ipynb
  • pyproject.toml

@AivanF AivanF force-pushed the aivan/2026-04-13-jafgen-dep branch from 1765a04 to 57296b9 Compare April 13, 2026 12:51
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
docs/examples/05_joins/joins_nb.ipynb (1)

20-20: ⚠️ Potential issue | 🟡 Minor

Fix malformed Markdown in prerequisites line.

Line 20 still has an extra trailing backtick after the parenthetical text, which breaks Markdown rendering.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/examples/05_joins/joins_nb.ipynb` at line 20, The prerequisites Markdown
line contains an extra trailing backtick in the string "**Prerequisites:** `pip
install motley-slayer[examples]` (jafgen is installed by the cell below)`" which
breaks rendering; edit that cell (the line containing the prerequisites string)
to remove the stray backtick at the end so it reads "**Prerequisites:** `pip
install motley-slayer[examples]` (jafgen is installed by the cell below)".
🧹 Nitpick comments (1)
docs/examples/05_joins/joins_nb.ipynb (1)

29-32: Prefer %pip over !pip in notebook install cells.

Line 32 should use %pip to ensure installation goes into the active Jupyter kernel environment.

Suggested change
-    "!pip install -q git+https://github.com/rossbowen/jaffle-shop-generator.git@09557a1118b000071f8171aa97d54d5029bf0f0b"
+    "%pip install -q git+https://github.com/rossbowen/jaffle-shop-generator.git@09557a1118b000071f8171aa97d54d5029bf0f0b"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/examples/05_joins/joins_nb.ipynb` around lines 29 - 32, Replace the
shell-style notebook install invocation using "!pip install -q
git+https://github.com/rossbowen/jaffle-shop-generator.git@09557a1118b000071f8171aa97d54d5029bf0f0b"
with a magic-style invocation using "%pip" so the package installs into the
active Jupyter kernel; update the notebook cell that contains the
jaffle-shop-generator install command to use "%pip install -q ..." instead of
leading "!" to ensure the installation targets the running kernel.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@docs/examples/05_joins/joins_nb.ipynb`:
- Line 20: The prerequisites Markdown line contains an extra trailing backtick
in the string "**Prerequisites:** `pip install motley-slayer[examples]` (jafgen
is installed by the cell below)`" which breaks rendering; edit that cell (the
line containing the prerequisites string) to remove the stray backtick at the
end so it reads "**Prerequisites:** `pip install motley-slayer[examples]`
(jafgen is installed by the cell below)".

---

Nitpick comments:
In `@docs/examples/05_joins/joins_nb.ipynb`:
- Around line 29-32: Replace the shell-style notebook install invocation using
"!pip install -q
git+https://github.com/rossbowen/jaffle-shop-generator.git@09557a1118b000071f8171aa97d54d5029bf0f0b"
with a magic-style invocation using "%pip" so the package installs into the
active Jupyter kernel; update the notebook cell that contains the
jaffle-shop-generator install command to use "%pip install -q ..." instead of
leading "!" to ensure the installation targets the running kernel.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d19aeb08-b41a-44c9-84e3-85fdf7f9aba4

📥 Commits

Reviewing files that changed from the base of the PR and between 1765a04 and 57296b9.

⛔ Files ignored due to path filters (1)
  • poetry.lock is excluded by !**/*.lock
📒 Files selected for processing (9)
  • .github/workflows/ci.yml
  • docs/examples/02_sql_vs_dsl/sql_vs_dsl_nb.ipynb
  • docs/examples/03_auto_ingest/auto_ingest_nb.ipynb
  • docs/examples/04_time/time_nb.ipynb
  • docs/examples/05_joined_measures/joined_measures_nb.ipynb
  • docs/examples/05_joins/joins_nb.ipynb
  • docs/examples/06_multistage_queries/multistage_queries_nb.ipynb
  • docs/examples/07_aggregations/aggregations_nb.ipynb
  • pyproject.toml
✅ Files skipped from review due to trivial changes (1)
  • docs/examples/04_time/time_nb.ipynb
🚧 Files skipped from review as they are similar to previous changes (6)
  • .github/workflows/ci.yml
  • docs/examples/06_multistage_queries/multistage_queries_nb.ipynb
  • docs/examples/03_auto_ingest/auto_ingest_nb.ipynb
  • docs/examples/05_joined_measures/joined_measures_nb.ipynb
  • docs/examples/07_aggregations/aggregations_nb.ipynb
  • docs/examples/02_sql_vs_dsl/sql_vs_dsl_nb.ipynb

AivanF added 2 commits April 13, 2026 18:50
PyPI rejects projects with dependencies outside of PyPI, and we need that specific commit (not merged still) that fixes a bug.
@AivanF AivanF force-pushed the aivan/2026-04-13-jafgen-dep branch from 57296b9 to 099bebf Compare April 13, 2026 15:52
@ZmeiGorynych
Copy link
Copy Markdown
Member

You committed the notebooks without the run results, can you please run the notebooks and commit them including execution results?

@AivanF AivanF merged commit d7bbae7 into main Apr 14, 2026
3 checks passed
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