fix: locate MEDS_transform-pipeline next to sys.executable (closes #51)#52
Open
mmcdermott wants to merge 1 commit into
Open
fix: locate MEDS_transform-pipeline next to sys.executable (closes #51)#52mmcdermott wants to merge 1 commit into
mmcdermott wants to merge 1 commit into
Conversation
…via PATH MIMIC_IV_MEDS/__main__.py shells out to the `MEDS_transform-pipeline` console script via `subprocess.run(["MEDS_transform-pipeline", ...])` (commands.py:39). The subprocess inherits the parent's PATH — but when the user runs the bundled `MEDS_extract-MIMIC_IV` script directly via its venv path WITHOUT first activating the venv, the venv's bin/ isn't on PATH, and the subprocess raises FileNotFoundError even though the script is installed in the same environment as the python interpreter that's calling it. Hit this in production after the download stage finally finished — pre_MEDS ran cleanly, then the MEDS extraction step died on the seam between the two because it couldn't find its own subprocess executable. Fix: introduce `resolve_console_script(name)` in commands.py that looks next to `sys.executable` first (where pip/uv guarantee console scripts land) and only falls back to a PATH lookup. Use it in __main__.py for the MEDS_transform-pipeline call. Tests: - test_resolve_console_script_prefers_sys_executable_dir — sibling wins over PATH - test_resolve_console_script_falls_back_to_path — apt-installed tools still work - test_resolve_console_script_raises_when_not_found — error names both lookup sites - test_resolve_console_script_actually_finds_real_console_script — self-check using MEDS_transform-pipeline from the test venv - test_resolve_console_script_skips_directories_named_like_scripts — `is_file()` guard against a directory shadowing the lookup - 2 doctests in commands.py covering happy + missing paths 29 tests + 3 doctests pass; ruff/format/docformatter clean. Closes #51.
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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
1 participant
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
Fix for #51 —
MIMIC_IV_MEDS/__main__.pyshells out to theMEDS_transform-pipelineconsole script viasubprocess.run(["MEDS_transform-pipeline", ...]). The subprocess inherits the parent'sPATH. When the user runs the bundledMEDS_extract-MIMIC_IVscript directly via its venv path without first activating the venv, the venv'sbin/is not onPATH, and the subprocess raisesFileNotFoundErroreven though the script is installed in the same environment as the python interpreter calling it.I just hit this in a real run — download stage finally succeeded after retries, pre_MEDS ran cleanly, then MEDS extraction died at the seam because
subprocesscouldn't find its own subprocess executable.Fix
Introduce
resolve_console_script(name)incommands.pythat looks next tosys.executablefirst (where pip / uv guarantee console scripts land), then falls back to a PATH lookup, then raises a usefulFileNotFoundErrornaming both sites tried. Use it in__main__.pyto resolveMEDS_transform-pipelineto an absolute path before invoking the subprocess.5 LOC in
__main__.py(one-line callsite change + import), ~25 LOC of helper + docstring incommands.py.Test plan
tests/test_commands.py— new file, 5 tests:test_resolve_console_script_prefers_sys_executable_dir— sibling wins over PATH (the load-bearing assertion)test_resolve_console_script_falls_back_to_path— system-installed tools still worktest_resolve_console_script_raises_when_not_found— error message names both lookup sitestest_resolve_console_script_actually_finds_real_console_script— self-check using theMEDS_transform-pipelinefrom the test venv (catches a busted test environment AND a busted real environment)test_resolve_console_script_skips_directories_named_like_scripts—is_file()guard against a directory shadowing the lookupcommands.pyfor happy + missing pathsNotes
commands.pyline exists in earlier versions; it's a long-standing PATH-coupling that only surfaces when the venv isn't activated. So no behavior change for users who already activate.shutil.whichfallback means tools intentionally placed elsewhere on PATH still work.subprocess).🤖 Generated with Claude Code