docs: fix tutorials/installation accuracy, move author tooling to the README - #257
docs: fix tutorials/installation accuracy, move author tooling to the README#257gtouloumes wants to merge 3 commits into
Conversation
dimitri-yatsenko
left a comment
There was a problem hiding this comment.
Really thorough, well-diagnosed PR — thank you. I checked each claim against how the docs pipeline actually works today, and the substance is correct and consistent with (indeed, improves) the existing process. Two things keep it from merging yet: it's out of date with several just-merged PRs, and one small wording inconsistency.
Verified against the current working process
- Credential mismatch is a real bug.
docker-compose.yamlsetsMYSQL_ROOT_PASSWORD: tutorial; the page'spasswordsnippet makes the firstdj.Schema()fail withAccess denied for user 'root'. I hit this exactroot/tutorialmismatch myself. Fix is right. - Graphviz/pydot is accurate. The render path is
pydot(a core dep) → the Graphvizdotexecutable, whichpipcan't supply;pip install … pygraphvizwas wrong. The docs image alreadyapk adds graphviz, which is why committed outputs always render. ✓ - The new
jupyterservice fits the existing architecture. It's behindprofiles: ["jupyter"], so a baredocker compose upand everyMODE=…invocation (LIVE/BUILD/EXECUTE/EXECUTE_PG) are untouched — I just ranMODE=EXECUTE_PGfor the notebook regeneration and this service would not have interfered. Read-only repo mount +/workvolume + MySQL-only, all documented. And becausepip_requirements.txtpullsdatajoint@master, the reader's live diagrams match the committed modernized outputs — no version skew. Nicely done.
Needs a rebase onto main before it can merge (currently CONFLICTING)
main has moved since this branch was cut. Overlaps to reconcile:
reference/specs/diagram.md— #247 rewrote the tier table + the Mermaid-output example just above your visualization-deps section. Your edit is complementary (different content), but it needs re-applying on top of #247.src/tutorials/basics/01-first-pipeline.ipynb— #255 replaced this notebook's Mermaid block with a generated SVG, and #256 (open) re-executes the whole notebook. Your one markdown-cell placeholder fix will collide. Cleanest order: let #256 land first, then rebase and re-apply just that cell..gitignore— #252 added.cache/; trivial tail conflict with your.ipynb_checkpoints/.src/index.md— different line from the recent figure edit; should auto-merge, just confirm after rebase.
One nit
The exception wording is inconsistent: the PR description says the render "raises DataJointError," while diagram.md and the new troubleshooting section say FileNotFoundError. A missing dot binary surfaces as FileNotFoundError, so the docs are right — please align the PR description (or confirm the actual exception and make all three agree). The troubleshooting anchor you link to is present and matches. 👍
Once rebased I'm happy to re-review and approve. Thanks again @gtouloumes — this is exactly the kind of new-developer-path cleanup the tutorials needed.
|
One precision on the mechanism, since the docs are close but don't name it: the diagrams in the notebooks render through So the framing here is exactly right: Graphviz is what the default |
There was a problem hiding this comment.
Thanks again, @gtouloumes — the accuracy fixes are keepers. Requesting changes to refocus the PR. This replaces my earlier premise wording (I'd overstated "read-only") — please read this version.
The model
Readers should be able to run the tutorials — but on their own infrastructure: they install DataJoint and connect to their own MySQL/PostgreSQL by following general setup instructions. What we do not ship or maintain is a documentation-specific runner. Our Docker / MODE=EXECUTE* tutorial stack exists to generate the published notebook outputs — that's author/contributor tooling, not a reader onramp. So: provide clear general setup instructions that let a reader self-run on their own DB, and drop the bundled reader-facing runner plus any framing that ties "running the tutorials" to our Docker infrastructure.
Requested structure
- Restore the minimal starting example (
Subject/Session/SessionAnalysis, placeholder credential) — the simplest end-to-end "getting started with DataJoint on your own database" snippet, independent of any tutorial infrastructure. - Documentation = general setup, simplest case.
installation.mdcovers installing and running DataJoint for the reader's own work (their own MySQL/PostgreSQL) and hosts the restored example. These are the instructions a reader follows to run the tutorials on their own infrastructure — not instructions for running inside our docs stack. - Author run/regenerate instructions → repo README. The Docker /
MODE=EXECUTE*"how to regenerate the documentation" steps are contributor tooling; keep them inREADME.md(where theMODE=…instructions already live) and remove the "Running the tutorials" section fromsrc/tutorials/index.md. The index stays read/navigational, and can link readers who want hands-on to the general setup in Installation. - Drop the bundled
jupyterCompose service as a documented path. A reader self-runs on their own infra via the setup instructions; they don't need our container. - Fix the landing-page card (
src/index.md:28): "hands-on Jupyter notebooks" invites the "run it via us" read — reword toward following worked pipelines (e.g. "Learn DataJoint through complete, worked Jupyter-notebook pipelines").
Keep as-is
datajoint[viz]/_repr_svg_-vs-draw()corrections and the GraphvizFileNotFoundErrortroubleshooting.installation.mdrestructure (Requirements up top, DataJoint.com ordering).- The
dj.Instancecredential-placeholder fix.
Net: readers can run the tutorials on their own DB by following the (general, simplest-case) setup instructions; we don't provide a docs-specific runner; author regeneration lives in the README.
|
Fully on board, thanks very much for the clarification! It boils down to some language confusion from the docs.datajoint.com landing page's Diataxis cards, where the Tutorials one reads "Build your first pipeline with hands-on Jupyter notebooks". I interpreted that as "the tutorials should be runnable". Agreed with the premise of being read-only for new developers. Will rebase and roll back to the requested structure |
|
Great — glad that clarifies it, and good catch on the source of the confusion. Since the Tutorials card wording ( |
|
One refinement before you rebase (I've updated the review above accordingly): readers should be able to run the tutorials — just on their own infrastructure, following general setup instructions (install DataJoint, point it at their own MySQL/PostgreSQL). So please don't strip the ability to self-run — keep clear, simplest-case setup instructions in Installation that enable it. What comes out is only the documentation-specific runner (the bundled |
`dj.Diagram`'s notebook display goes `_repr_svg_()` -> `make_svg()` -> `make_dot()` -> pydot -> the Graphviz `dot` binary. pydot is a core dependency; Graphviz is a system package that pip cannot install. The spec instead said `pip install matplotlib pygraphviz`, which supplies neither the render path nor the binary. matplotlib is only needed for `Diagram.draw()`, the separate `make_image()` path that most usage never touches. Also drops the claim that missing dependencies yield a warning and a stub class -- the render call raises `FileNotFoundError`, which the Installation page now documents as a troubleshooting symptom.
…Joint Installation now covers what a reader needs to run DataJoint -- and the tutorial notebooks -- on their own infrastructure, rather than pointing at the documentation's own Docker stack. - Requirements moves to the top: it describes DataJoint generally, not the hosted options it was buried under. DataJoint.com moves above Local Development, matching its "Recommended" label. - Adds "Your First Pipeline", the Subject/Session/SessionAnalysis snippet that previously opened the Tutorials page. It is the simplest end-to-end path against a reader's own database, independent of any tutorial infrastructure. Credentials are placeholders; the old copy wrote a literal password that also happened to be wrong for the tutorial container. - Adds "Running the Tutorial Notebooks": Jupyter, where to get the notebooks, and how DataJoint resolves datajoint.json and .secrets/ from them. - Corrects the `datajoint[viz]` comment, which advertised "diagram visualization" but installs matplotlib + ipython -- that serves `Diagram.draw()`, not the notebook SVG path. - Documents Graphviz as a troubleshooting symptom rather than a prerequisite: DataJoint imports, connects, defines, inserts, queries, and computes without it.
…the README
The Docker / MODE=EXECUTE* stack exists to generate the published notebook
outputs -- it is contributor tooling, not a reader onramp. The tutorials index
carried it as reader setup ('docker compose up -d' plus a host 'jupyter lab'),
which is neither the intended path nor a complete one.
That section comes out, and so does the Quick Start above it: a standalone
bring-your-own-database walkthrough sitting where step one of a tutorial flow
would be. It now lives in installation.md. The index keeps its navigational
job and links readers who want hands-on to the general setup there.
The README's Notebook execution policy gains the environment facts the docs
body had been carrying: the Compose credentials, why the image supplies
Graphviz, and that pip_requirements.txt tracks datajoint@master.
src/index.md: the Tutorials card promised 'hands-on Jupyter notebooks', which
reads as a runner we ship rather than notebooks a reader runs on their own
database -- the misreading this PR originally acted on. The footer link named
a Quick Start section that no longer exists on the page it pointed to.
0cc62fa to
9034a54
Compare
|
Rebased to main, all changes address, and PR title and body updated accordingly. Ready for re-review @dimitri-yatsenko! |
dimitri-yatsenko
left a comment
There was a problem hiding this comment.
The rework matches the agreed structure exactly — thanks for turning it around so cleanly, @gtouloumes:
- Minimal starting example restored in
installation.md(Subject/Session/SessionAnalysis + populate), framed as the general own-database quickstart. tutorials/index.mdback to read/navigational; author run/regenerate tooling moved to the README; the bundled jupyter runner and reader-run framing removed.- Landing-page card reworded to "complete, worked Jupyter-notebook pipelines."
- Diagram visualization deps corrected (pydot → Graphviz
dot; matplotlib only fordraw()).
LGTM.
What
Fixes the accuracy of the Tutorials and Installation pages, makes Installation the general
setup path a reader follows to run DataJoint — and the tutorial notebooks — on their own
infrastructure, and moves author regeneration tooling to the repository README.
Rebased onto
main(df7282e5) and re-authored as three commits, one concern each.Why
The original version of this PR was built on a premise @dimitri-yatsenko corrected in review: it
treated "readers cannot execute the notebooks" as the problem and shipped a documentation-specific
runner to solve it. The actual model is that readers run the tutorials on their own
infrastructure, by following general setup instructions — and our Docker /
MODE=EXECUTE*stack exists to generate the published outputs, which makes it contributor tooling. So the
bundled runner is gone, and Installation now carries the setup that enables self-running.
The underlying accuracy problems the PR found are unchanged and still worth fixing:
Credentials
The Tutorials page wrote
passwordinto.secrets/database.passwordwhiledocker-compose.yamlsets
MYSQL_ROOT_PASSWORD: tutorial. Followed exactly, the first cell callingdj.Schema()failswith
OperationalError: (1045, "Access denied for user 'root'"). Those credentials belong to ourCompose stack, so they now live in the README with the rest of the author tooling; the reader-facing
snippet in Installation uses placeholders.
Undocumented Graphviz dependency for
dj.Diagramdj.Diagram(schema)— cell 12 of the first tutorial, and 22 files acrosstutorials/andhow-to/— raisesFileNotFoundErroron any machine without Graphviz. The only mention outsidethe docs image was one line in the diagram spec reading
pip install matplotlib pygraphviz. Thenotebook display path is
_repr_svg_()→make_svg()→make_dot()→ pydot (a coredependency) → the Graphviz
dotbinary;pipcannot supplydoteither way, and matplotlibserves only the separate
Diagram.draw()/make_image()path. The docs image installs Graphvizvia
apk, so committed outputs always show clean diagrams regardless of what a reader can renderlocally.
Setup instructions were incomplete and out of order
The page opened with a Quick Start that read as step one of a Docker flow but was actually a
standalone bring-your-own-database walkthrough. The real setup sat at the bottom and omitted half
of what it needed:
docker compose up -dstarts only the databases,jupyter labruns on thehost, and no local Python or Jupyter install was mentioned anywhere.
Changes
how-to/installation.md— now the general setup path.buried under. DataJoint.com above Local Development, matching its "Recommended" label.
Subject/Session/SessionAnalysissnippet restored from the oldTutorials Quick Start, with placeholder credentials. The simplest end-to-end path against a
reader's own database, independent of any tutorial infrastructure.
clone), and how DataJoint resolves
datajoint.jsonand.secrets/from a notebook's directory.Infrastructure-neutral: nothing here depends on our Compose stack.
connects, defines, inserts, queries, and computes without it.
datajoint[viz]comment, which advertised "diagram visualization" but installsmatplotlib + ipython.
tutorials/index.md— back to navigational. Quick Start moved to Installation; thereader-facing "Running the Tutorials" section removed. The intro states that notebooks are
published with their executed outputs, and links readers who want hands-on to the general setup.
reference/specs/diagram.md— replaced thepygraphvizinstruction, named the_repr_svg_render path per @dimitri-yatsenko's note so it is unambiguous which path most usage hits, and
dropped the claim that missing dependencies yield a warning and a stub class.
README.md— theMODE=EXECUTE*instructions already lived under Notebook execution policy;they gain the environment facts the docs body had been carrying: the Compose credentials
(
root/tutorialfor MySQL,postgres/tutorialfor PostgreSQL), why the image suppliesGraphviz, and that
pip_requirements.txttracksdatajoint@master.src/index.md— the Tutorials card promised "hands-on Jupyter notebooks", which reads as arunner we ship rather than notebooks a reader runs on their own database. This is the wording that
prompted this PR's original framing, so it is fixed here. The footer link also named a Quick Start
section that no longer exists on the page it pointed to.
Not in this PR
dj.Instancecredential placeholder in01-first-pipeline.ipynb. docs: regenerate all notebooks against the modernized dj.Diagram renderer #256 regenerates thatnotebook wholesale, so per review it is held back rather than dropped — it lands as a one-line
follow-up once docs: regenerate all notebooks against the modernized dj.Diagram renderer #256 merges.
jupyterCompose service,jupyter_workvolume, and thejupyterlabrequirement.Removed entirely.
docker-compose.yaml,pip_requirements.txt, and.gitignoreare nowbyte-identical to
main.Verification
MODE="BUILD" docker compose up --buildexits 0. Both new cross-references resolve(
installation.md#running-the-tutorial-notebooks,installation.md#djdiagram-raises-filenotfounderror);the remaining anchor notices are pre-existing and in untouched files, and the stderr warnings are
MkDocs' own deprecation message.
The restored minimal example was run end to end in the docs image against a scratch MySQL:
DataJoint 2.3.3.dev12 connected,
populate()returned[{'subject_id': 1, 'session_idx': 1, 'result': 42.0}].Correction to my original description: it said the render raises
DataJointError. A missingdotbinary surfaces as
FileNotFoundError— the docs were right and the description was wrong.