Repository navigation
Conversation
- Sync tutorial_pyhealth_{data,trainer,medcode}.ipynb with the Colab
versions linked from docs/tutorials.rst. The repo copies of data and
trainer still used dataset.patients, which BaseDataset no longer has
(it is unique_patient_ids). The medcode notebook's ICD9CM->ICD10CM
cell relies on the icd-mappings backend added in the parent commits.
- docs/tutorials.rst: link each Colab tutorial to its source notebook,
turn every example filename into a link to the real file (fixing
wrong or renamed paths such as the EEG SparcNet and eICU scripts),
fix the broken readmission list-table, and blob/main -> blob/master.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI review: findings
This docs-and-notebooks PR re-lands the Colab sync for three tutorial notebooks and rewrites docs/tutorials.rst so every example entry links to a [Source] file. I checked that every linked example and tutorial path exists in the tree, and that the APIs the notebooks now call (unique_patient_ids, get_patient, global_event_df with an event_type column) exist in pyhealth/datasets/base_dataset.py and pyhealth/data/data.py. The medcode notebook already uses CrossMap.load("ICD9CM", "ICD10CM") from #1257, and the old dataset.patients attribute is gone, so the notebook fix is needed. Blocking: docs/tutorials.rst changes the datasets_overview link from blob/main to blob/master, and about 80 links now depend on a master branch. The environment reports main as the main branch and the clone has no refs to confirm master holds these files, so the links could 404. Non-blocking: the data notebook hardcodes unverified dataset counts in a comment, and the medcode notebook adds an unrelated sorted() change. No public API, base class, or core code changes, and no tests are expected for notebook content. Overall risk is low once the branch-name question is settled. Redundancy: none.
Reviewed commit 7935b8e
| ======================= | ||
|
|
||
| For information on how to access and download the datasets supported by PyHealth, please refer to our `Datasets Overview Notebook <https://github.com/sunlabuiuc/PyHealth/blob/main/examples/datasets_overview.ipynb>`_. | ||
| For information on how to access and download the datasets supported by PyHealth, please refer to our `Datasets Overview Notebook <https://github.com/sunlabuiuc/PyHealth/blob/master/examples/datasets_overview.ipynb>`_. |
There was a problem hiding this comment.
[medium] Links switched from blob/main to blob/master without confirming master exists
The datasets_overview link changes from blob/main to blob/master, and all new [Source] and example links use blob/master too. The environment reports 'main' as the repository's main branch, and the local clone has no refs to show that a master branch exists or carries these files. If master is not the GitHub default or does not contain these paths, roughly 80 links in this file return 404. Confirm on GitHub that master contains examples/tutorials/*.ipynb and the example scripts, or revert to main, which the original link used.
| "outputs": [] | ||
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": "from pyhealth.datasets import MIMIC3Dataset\nimport polars as pl\n\nroot = \"https://storage.googleapis.com/pyhealth/Synthetic_MIMIC-III\"\n\ndataset = MIMIC3Dataset(\n root=root,\n dataset_name=\"mimic3\",\n tables=[\n \"diagnoses_icd\", # ICD-9 diagnosis codes per admission\n \"prescriptions\", # medication orders\n \"noteevents\", # clinical notes\n ],\n)\n\nprint(f\"Loaded {len(dataset.unique_patient_ids)} patients\")\n\n# Pick a patient who actually has events from all three tables, so every query\n# below returns something. Notes are sparse in this synthetic set (~1k notes\n# across ~50k patients), so patient 0 almost certainly has none.\npatient_ids_with_notes = (\n dataset.global_event_df\n .filter(pl.col(\"event_type\") == \"noteevents\")\n .select(\"patient_id\")\n .collect()[\"patient_id\"]\n .unique()\n)\npatient_id = patient_ids_with_notes[0]\npatient = dataset.get_patient(patient_id)\n\nprint(f\"\\nPatient ID: {patient.patient_id}\")\nprint(f\"Event types present: {list(patient.event_type_partitions.keys())}\")\nprint(f\"Total events: {len(patient.get_events())}\")" |
There was a problem hiding this comment.
[low] Hardcoded dataset statistics in code comment are unverified
The comment in the patient-selection cell says notes are sparse, with '~1k notes across ~50k patients', and that patient 0 'almost certainly has none'. Neither figure is checked in this PR. Remove the numbers or compute them in the notebook. Note also that unique() on the note patient IDs is unordered, so picking index 0 is not deterministic across runs.
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": "# Pattern 2: Translate a patient's ICD-9 code list to CCS before modeling\n# This dramatically reduces vocabulary size.\n\ndef translate_codes_to_ccs(icd9_codes, crossmap):\n \"\"\"Map a list of ICD-9 codes to CCS categories.\"\"\"\n ccs_codes = []\n for code in icd9_codes:\n mapped = crossmap.map(code)\n ccs_codes.extend(mapped)\n return list(set(ccs_codes)) # deduplicate\n\n# Example patient from MIMIC-III\npatient_icd9_codes = [\"250.00\", \"401.9\", \"428.0\", \"585.3\"]\n# = Type 2 DM, Essential hypertension, Heart failure, CKD stage 3\n\npatient_ccs_codes = translate_codes_to_ccs(patient_icd9_codes, cm_9toCCS)\nprint(\"ICD-9 codes:\", patient_icd9_codes)\nprint(\"CCS codes: \", patient_ccs_codes)\nprint(f\"Vocabulary reduction: {len(icd9cm.graph.nodes)} ICD-9 codes \u2192 ~300 CCS categories\")" | ||
| "source": "# Pattern 2: Translate a patient's ICD-9 code list to CCS before modeling\n# This dramatically reduces vocabulary size.\n\ndef translate_codes_to_ccs(icd9_codes, crossmap):\n \"\"\"Map a list of ICD-9 codes to CCS categories.\"\"\"\n ccs_codes = []\n for code in icd9_codes:\n mapped = crossmap.map(code)\n ccs_codes.extend(mapped)\n return sorted(set(ccs_codes)) # deduplicate (sorted: set order is not stable)\n\n# Example patient from MIMIC-III\npatient_icd9_codes = [\"250.00\", \"401.9\", \"428.0\", \"585.3\"]\n# = Type 2 DM, Essential hypertension, Heart failure, CKD stage 3\n\npatient_ccs_codes = translate_codes_to_ccs(patient_icd9_codes, cm_9toCCS)\nprint(\"ICD-9 codes:\", patient_icd9_codes)\nprint(\"CCS codes: \", patient_ccs_codes)\nprint(f\"Vocabulary reduction: {len(icd9cm.graph.nodes)} ICD-9 codes \u2192 ~300 CCS categories\")" |
There was a problem hiding this comment.
[low] Unrelated behaviour change: sorted() added to translate_codes_to_ccs
The PR description says the medcode notebook is only synced to Colab. This hunk also changes the helper's return value from list(set(...)) to sorted(set(...)). The change is harmless and makes output stable, but it is an undocumented change to the example's code. Either mention it in the PR description or drop it.
There was a problem hiding this comment.
AI Review
suggestion: human review
decision: not available
Thank you for re-landing this work. Syncing the three tutorial notebooks with Colab and adding working source links across docs/tutorials.rst will make the tutorials much easier to use. CI coverage data wasn't available for this run, so the coverage bar couldn't be checked automatically. A maintainer will take a look and confirm. They will also check that the blob/master links match the repository's default branch name. Nothing here needs action from you right now.
Reviewed commit 7935b8e
Re-lands #1256 on
master. #1256 was merged intomedcode/icd-mappingstwo minutes after that branch had already been squash-merged as #1257, so its changes never reachedmaster. This is the same commit cherry-picked ontomaster, with identical content and no conflicts.Changes
examples/tutorials/tutorial_pyhealth_medcode.ipynb: now matches the Colab version. ItsICD9CM → ICD10CMcell uses theicd-mappingsbackend from medcode: ICD-9 <-> ICD-10 translation and grouper vocabularies via icd-mappings #1257.tutorial_pyhealth_data.ipynb,tutorial_pyhealth_trainer.ipynb: synced with Colab, replacing the removeddataset.patientswithunique_patient_ids.docs/tutorials.rst:[Source]links to each tutorial notebook.blob/mainbecomesblob/master, and the file has no docutils warnings.Testing
!pip, withicd-mappings0.6.2; all pass. The data and trainer notebooks passed againstmasterin Sync tutorial notebooks with Colab and link example code in docs #1256, and nothing they depend on has changed since.🤖 Generated with Claude Code