Skip to content

issue(medcat-tutorials): CU-869c87xpy Make sure all the tests run against current state of core lib#350

Merged
mart-r merged 12 commits intomainfrom
issue/medcat-tutorials/CU-869c87xpy-fix-running-against-current-state
Feb 26, 2026
Merged

issue(medcat-tutorials): CU-869c87xpy Make sure all the tests run against current state of core lib#350
mart-r merged 12 commits intomainfrom
issue/medcat-tutorials/CU-869c87xpy-fix-running-against-current-state

Conversation

@mart-r
Copy link
Collaborator

@mart-r mart-r commented Feb 23, 2026

Turns out the patching system introduced in #27 was merged in some kind of debug state where it didn't actually do what it was supposed to.
That meant that the tutorials were never actually being tested against the current state of the core lib.

So this PR fixes that.

PS:
After this is done, I want to do a follow-up PR for #333 and #343 such that the script there uses the script here to bump the install targets in notebooks as well.

NOTE: Should be good to go after #349 is merged into this one Done now

EDIT:
Added a check for the script that fails if there's not enough changes through the notebook patcher.
So it fails if you have too big a number:

% python .ci/patch_notebook_installs.py . -c 5 
[PATCHED] notebooks/introductory/meta/1._Add_a_MetaCat_to_a_Model.ipynb
 with: '! pip install \"/Users/martratas/Documents/CogStack/.MedCAT.nosync/monorepo-nlp/medcat-v2[meta-cat]\"'
[PATCHED] notebooks/introductory/relcat/1._Supervised_Training_Relation_Extraction.ipynb
 with: '! pip install \"/Users/martratas/Documents/CogStack/.MedCAT.nosync/monorepo-nlp/medcat-v2[spacy,rel-cat,meta-cat]\"'
[PATCHED] notebooks/introductory/relcat/2._Infering_relations_from_annotations_with_Relation_toolkit.ipynb
 with: '! pip install \"/Users/martratas/Documents/CogStack/.MedCAT.nosync/monorepo-nlp/medcat-v2[spacy,rel-cat]\"'
[PATCHED] notebooks/introductory/migration/1._Migrate_v1_model_to_v2.ipynb
 with: '! pip install \"/Users/martratas/Documents/CogStack/.MedCAT.nosync/monorepo-nlp/medcat-v2[meta-cat,spacy,deid]\"'
Expected a minimum of 5 changes,but only found 4 changes. This will force a non-zero exit status so GHA workflow can fail
% echo $?
1

But is successful otherwise:

% python .ci/patch_notebook_installs.py . -c 4
[PATCHED] notebooks/introductory/meta/1._Add_a_MetaCat_to_a_Model.ipynb
 with: '! pip install \"/Users/martratas/Documents/CogStack/.MedCAT.nosync/monorepo-nlp/medcat-v2[meta-cat]\"'
[PATCHED] notebooks/introductory/relcat/1._Supervised_Training_Relation_Extraction.ipynb
 with: '! pip install \"/Users/martratas/Documents/CogStack/.MedCAT.nosync/monorepo-nlp/medcat-v2[spacy,rel-cat,meta-cat]\"'
[PATCHED] notebooks/introductory/relcat/2._Infering_relations_from_annotations_with_Relation_toolkit.ipynb
 with: '! pip install \"/Users/martratas/Documents/CogStack/.MedCAT.nosync/monorepo-nlp/medcat-v2[spacy,rel-cat]\"'
[PATCHED] notebooks/introductory/migration/1._Migrate_v1_model_to_v2.ipynb
 with: '! pip install \"/Users/martratas/Documents/CogStack/.MedCAT.nosync/monorepo-nlp/medcat-v2[meta-cat,spacy,deid]\"'
% echo $?                                     
0

@tomolopolis
Copy link
Member

tomolopolis commented Feb 23, 2026

@mart-r mart-r marked this pull request as draft February 23, 2026 16:24
@mart-r mart-r changed the title issue(medcat-tutorials): CU-869c87tt9 Make sure all the tests run against current state of core lib issue(medcat-tutorials): CU-869c87xpy Make sure all the tests run against current state of core lib Feb 23, 2026
@mart-r mart-r marked this pull request as ready for review February 23, 2026 19:58
Copy link
Collaborator

@alhendrickson alhendrickson left a comment

Choose a reason for hiding this comment

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

Assuming this regex works - lgtm

- name: Update install targets in notebooks
run: |
python .ci/patch_notebook_installs.py .
git status
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can probably remove these now, assuming they were for debugging

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, should do.

Though I think it would be good to include something that makes this fail if there's nothing changed. I've got a decent idea for that.

@mart-r mart-r merged commit 276a3b7 into main Feb 26, 2026
15 checks passed
@mart-r mart-r deleted the issue/medcat-tutorials/CU-869c87xpy-fix-running-against-current-state branch February 26, 2026 13:02
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.

3 participants