Restore test collection and lint cleanliness after the Task B merge - #87
Merged
Conversation
tests/test_valvular_probes.py imported scripts/extract_valvular_labels.py at module scope, which imports medspacy and builds the full NER pipeline — neither medspacy nor the extraction stack is a declared dependency, so pytest could not collect the suite on a machine without them. The import is now guarded and only the one NER test skips when medspacy is absent. Also clears the 55 ruff errors and 10 unformatted files the Task B PRs left on main: ruff --fix + format across the valvular files, noqa on the deliberately late imports, and removal of two unused local assignments. 132 passed, 1 skipped; ruff check and format --check both clean.
duckyquang
requested review from
gullyboyslok,
kevzho,
noahdanehebdon,
paiksca,
sebasmos,
tigerkrittaphas and
turkalpmd
September 9, 2026 13:53
sebasmos
approved these changes
Sep 9, 2026
sebasmos
left a comment
Member
There was a problem hiding this comment.
Read the diff and verified both behavioral claims against the code: the import guard and skip are scoped to exactly one test, the two removed assignments are confirmed dead code. Real run: 132 passed, 1 skipped, ruff clean. APPROVE.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Main stopped collecting tests for anyone without medspacy —
tests/test_valvular_probes.pyimportsscripts/extract_valvular_labels.pyat module scope, which imports medspacy and builds the full NER pipeline on import, and neither medspacy nor the extraction stack is a declared dependency. The import is now guarded, so only the one NER test (test_clinical_ner_extraction_negation_and_severity) skips when medspacy is missing; the other valvular tests keep running everywhere.While in there I cleared the lint debt the Task B merge left on main: 55 ruff errors and 10 unformatted files, which means
make lint/ pre-commit was red for everyone. Mechanics:ruff check --fix+ruff formatacross the valvular files,# noqa: E402on the deliberately-late imports (they sit afterload_dotenv()/logging setup on purpose), and two unused local assignments removed (y_dict_va,confidence_dist— pure computations, no side effects).Result
132 passed, 1 skipped(the NER test, on my machine without medspacy);ruff checkandruff format --checkboth fully clean. Real run on my Mac with the repo venv.Notes
task-boptional-dependencies group inpyproject.tomlas a follow-up, so the skip behavior is documented rather than accidental.