Repository navigation
docs: link the tutorial series to the PyHealth 2.1 Colab notebooks - #1311
Conversation
The tutorials page opened the pre-2.1 notebook copies in examples/tutorials/series. Point it at the maintained 2.1 Colab notebooks (the same ones listed on pyhealth.dev), add the three What's new in 2.1 notebooks, and note that Tutorials 04 and 07 open on a T4 GPU. Also fix two existing RST errors on the page: a mis-indented row that broke the readmission examples table, and a comment that swallowed the next heading. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Test CoverageTotal: 69.05% (17723 / 25668 lines)
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PhbzTePoLMBYwjMLG8R9ek
|
@jhnwu3 could you review and approve this when you get a chance? It only changes |
|
/review |
There was a problem hiding this comment.
Previous review
AI review: findings
Docs-only change to docs/tutorials.rst. It repoints the 00–08 tutorial links to the 2.1 Colab notebooks, updates the 04 and 07 descriptions, adds a "What's new in PyHealth 2.1" table for three notebooks, and fixes two RST errors (a mis-indented readmission table row and a commented-out Pipeline 5 line that swallowed a heading). I checked the claims against the repo: pyproject.toml is 2.1.0, the local notebooks install pyhealth==2.1.0, notebooks 00, 04 and 07 carry T4 GPU metadata, and PatientSplit, set_pos_weight, CCSR, nested_multihot, XGBoostModel and TreeSHAP all exist, so the feature names on the page are accurate. No code is added, so nothing is redundant with existing components. Blocking (medium): the "What's new" intro says the three notebooks "walk through every 2.1 change", but CHANGELOG.md's 2.1.0 section also lists the Synthea model, the HALO vectorisation and the Trainer best-epoch change, which none of the three notebooks cover. Non-blocking (low): the Colab Drive IDs cannot be checked from the repository, since the local notebooks are the only in-repo copies. Testing is a docutils parse, which is adequate for an RST-only edit. Overall risk is low; the one fix needed is rewording the overclaim.
Reviewed commit d4bd9f8
There was a problem hiding this comment.
Previous review
AI Review
suggestion: need change
decision: need change
Thank you for this PR. Pointing the tutorials page at the maintained 2.1 Colab notebooks, adding the "What's new in PyHealth 2.1" section, and fixing the two RST errors (the mis-indented table row and the swallowed heading) all make the page better. It is close. Coverage isn't a concern for this docs-only change: the whole-repo total is 67.88%, above the 60% bar, and no files under pyhealth/ changed.
To get this merged:
- Reword the intro of the "What's new in PyHealth 2.1" section. It says the three notebooks "walk through every 2.1 change", but the 2.1.0 section of CHANGELOG.md also lists changes that none of the three cover: the Synthea model, the HALO vectorisation and the Trainer best-epoch change. Please either soften the sentence (for example, "walk through the main 2.1 changes on runnable examples") or add coverage of those items. Keeping the link to the release notes for the complete list would help.
Optional suggestions:
- The Colab Drive links can't be checked from the repository. Please open each one once, in a signed-out browser, to confirm it is shared publicly and installs
pyhealth==2.1.0.
Reviewed commit d4bd9f8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the review. Fixed in 8986d28: the intro now says the three notebooks "walk through the main 2.1 features on runnable examples", and points to the release notes for the full list of changes. On the optional point, all twelve Colab links on the page (tutorials 00–08 and the three 2.1 notebooks) open signed-out. Each returns 200 from |
|
/review |
There was a problem hiding this comment.
AI review: findings
This docs-only PR changes docs/tutorials.rst. It repoints tutorials 00-08 from the GitHub-hosted notebooks in examples/tutorials/series to the maintained 2.1 Colab copies, updates the 04 and 07 descriptions, and adds a "What's new in PyHealth 2.1" table of three notebooks. It also fixes an RST indentation error in the readmission table and a commented-out link that swallowed the "Multimodal & Smart Processors" heading. The Colab IDs for tutorials 00-08 match the cross-links inside the repo's notebooks, and the T4 GPU and model-list claims match examples/tutorials/series/README.md. Every API the new section names exists: PatientSplit with four ratios, BaseModel.set_pos_weight, bootstrap_ci, the calibration metrics listed in CHANGELOG.md, XGBoostModel with TreeSHAP, CCSR mapping, and nested_multihot. Nothing is redundant, since no code is added. There are no code or test changes, and the author reports a docutils parse, which I could not re-run here. Non-blocking: the three "What's new" Colab IDs and the v2.1.0 release-notes URL appear nowhere else in the repo, so someone should open them before merge. Overall risk is low, with no blocking problems found.
Reviewed commit 8986d28
|
|
||
| * - Notebook | ||
| - What you will learn | ||
| * - `1/3. Splits you can trust, runs you can repeat <https://colab.research.google.com/drive/1yqgMaVuGWw6IGjDi6L_S7MHp087A2fWg>`_ |
There was a problem hiding this comment.
[low] Three "What's new" Colab links are not verified anywhere in the repo
The Colab IDs for the three new 2.1 notebooks (lines 53, 55, 57) appear only in this file. The tutorial series links are cross-linked from the repo's own notebooks, but the whats_new_*.ipynb files and examples/tutorials/series/README.md do not link to these Colab copies. The README points readers to pyhealth.dev for the maintained copies instead. Before merge, open each link and confirm it goes to the matching 2.1 notebook. Consider adding the same links to the whats_new notebooks or the README so they can be checked in the repo.
| Already using PyHealth? Three short notebooks, about 10 minutes each on a free | ||
| CPU runtime, walk through the main 2.1 features on runnable examples. For the | ||
| full list of changes, see the | ||
| `release notes <https://github.com/sunlabuiuc/PyHealth/releases/tag/v2.1.0>`_. |
There was a problem hiding this comment.
[low] Release-notes URL for v2.1.0 is not referenced elsewhere in the repo
The link to https://github.com/sunlabuiuc/PyHealth/releases/tag/v2.1.0 is not found in the repo; CHANGELOG.md does list the 2.1 features. Confirm that the GitHub release tag exists before merge so the link does not 404.
There was a problem hiding this comment.
AI Review
suggestion: approve
decision: approve
Thank you for this update. Pointing the tutorials page at the maintained PyHealth 2.1 Colab notebooks is useful. The new "What's new in PyHealth 2.1" section and the two RST fixes (the mis-indented readmission table row and the commented-out Pipeline 5 link that swallowed a heading) are welcome too.
The diff only changes docs/tutorials.rst, matching your description, and I found no problems with the edits. The descriptions for 04 and 07 and the API names in the new section look consistent with the library.
Coverage: The whole repository is at 69.05%, above the 60% bar. The coverage report lists some pyhealth/ files, such as metrics/multiclass.py at 25% and metrics/calibration.py at 65%. This PR doesn't touch them, so they don't count against it. A docs-only change needs no new tests.
This looks ready to merge.
Optional suggestions:
- Before merging, open the three new "What's new" Colab links and the v2.1.0 release-notes URL. I can't check from the diff that they are public and load correctly.
- Make sure each Colab notebook is shared as "anyone with the link can view", so readers don't hit a permissions wall.
Reviewed commit 8986d28
Summary
The tutorials page opens the notebook copies in
examples/tutorials/series, which predate the 2.1.0 release (they install PyHealth from GitHub and miss the fixes since). This points the page at the maintained 2.1 Colab notebooks, the same ones listed on pyhealth.dev:pyhealth==2.1.0. The descriptions for 04 and 07 are updated: 04 compares six models, 07 adds label distributions and baseline checks, and both open on a free T4 GPU.The notebooks in
examples/tutorials/seriesare unchanged; this PR only changesdocs/tutorials.rst.Testing
docs/tutorials.rstwith docutils: no errors. The only remaining warning is the pre-existing transition at the end of the file.tools/check_pr_rules.pypasses.🤖 Generated with Claude Code