Skip to content

fix(python): launch phy and suite2p through uv instead of .BAT wrappers - #3

Closed
tabedzki wants to merge 1 commit into
feat/uv-python-envfrom
tabedzki/fix/uv-phy-launcher
Closed

tabedzki wants to merge 1 commit into
feat/uv-python-envfrom
tabedzki/fix/uv-phy-launcher

Conversation

@tabedzki

Copy link
Copy Markdown
Contributor

🤖 Human guided, AI assisted PR (using this skill). AI text below. 🤖

🤖 AI text below 🤖

Stacked on #2 — targets feat/uv-python-env, not main.

The bug

Clicking Open Phy on macOS failed with:

zsh:1: permission denied: PythonScripts/open_phy.BAT

open_phy.BAT and open_suite2p.BAT are cmd.exe batch files, so MATLAB's system() handed them to zsh, which tried to execute a non-executable Windows file. Setting the execute bit would not have helped either — @echo off and call conda activate are not shell commands.

Both wrappers also assumed phy and suite2p lived in a conda env named on the command line (iblenv). That does not hold in practice: on the reporting machine phy is installed in a separate phy2 env, and iblenv has no phy at all.

The fix

Replace both .BAT wrappers with standalone launchers that declare their dependencies inline (PEP 723) and run under uv — which the app already locates at startup for the parameter helper scripts:

uv run --no-project PythonScripts/open_phy.py <sorting_output_dir>
  • PythonScripts/open_phy.pyphy==2.1.0
  • PythonScripts/open_suite2p.pysuite2p[gui]==1.1.0

Both are pinned from PyPI, so neither tool needs conda now. They are deliberately kept out of the root pyproject.toml: phy pins numpy<2 and pulls its own Qt stack, which cannot coexist with the >=3.14 helper-script environment. uv resolves each into its own cached environment on first use.

OpenExtGUI.m now builds the uv command from a new app.py_uv, set alongside app.py_env in getPythonEnv.m, and reports a clear message when uv is unavailable rather than launching nothing.

Cross-platform uv discovery

findUv never assumed a fixed path, but MATLAB's system() inherits a minimal PATH, which was the real failure mode. On unix it now probes $SHELL -l -c first, sourcing the user's profile:

minimal PATH, plain probe:        NOT FOUND
minimal PATH, login-shell probe:  /opt/homebrew/bin/uv

Fallback install locations now also cover linuxbrew, /usr/bin and /snap/bin.

Drive-by bug fix

In OpenExtGUI, when system() threw, out was never assigned, so the following if out ~= 0 raised its own error and masked the real failure. The exit-status check moved inside the try block.

Verification

  • phy 2.1.0 and suite2p 1.1.0 both install and import under uv on macOS; phy --help lists template-gui
  • The exact MATLAB-style command runs under a stripped env -i environment
  • Paths containing spaces parse correctly (phy gets past params.py into array loading)
  • find_stat resolves all three suite2p nesting levels; an empty dir opens the GUI empty
  • checkcode on all three modified .m files reports 0 syntax errors (the one remaining lint is a pre-existing unused event callback argument)

suite2p backwards compatibility

Confirmed, since it decided the pin. A legacy-format plane0 directory (ops/stat/F/Fneu/spks/iscell written with allow_pickle) was loaded through suite2p 1.1.0's real GUI entry point, suite2p.gui.io.load_files. All arrays returned correctly, and a missing optional redcell.npy degraded to "no channel 2" rather than erroring. Pinning the current release does not strand existing results.

Not included

  • OpenExtGUI2 (IBL atlas GUI) still uses the iblenv conda env — migrating iblapps-master is a larger change. Note it is currently broken on macOS for a related reason: getCondaEnvPython matches the Name column of conda env list, which is blank when envs live outside the standard envs dir, so app.py_ibl_env resolves to [].
  • Neither GUI window was launched to confirm it paints, as no real sorting output was available. The launch path is verified up to the point where phy loads data.

Assisted-by: ClaudeCode:claude-opus-5

🤖 Generated with Claude Code

The "Open Phy" button failed on macOS with

    zsh:1: permission denied: PythonScripts/open_phy.BAT

open_phy.BAT and open_suite2p.BAT are cmd.exe batch files, so MATLAB's
system() handed them to zsh, which tried to execute a non-executable
Windows file. Marking them executable would not have helped either, since
`@echo off` and `call conda activate` are not shell commands.

Both wrappers also assumed phy and suite2p lived in a conda env named on
the command line (iblenv). That assumption does not hold in practice: on
the reporting machine phy is installed in a separate `phy2` env and
iblenv has no phy at all.

Replace both with standalone launchers that declare their dependencies
inline (PEP 723) and run under uv, which the app already locates at
startup for the parameter helper scripts:

    uv run --no-project PythonScripts/open_phy.py <sorting_output_dir>

phy (2.1.0) and suite2p (1.1.0) are both pinned from PyPI, so neither
tool needs conda now. They are deliberately kept out of the root
pyproject.toml: phy pins numpy<2 and pulls its own Qt stack, which cannot
coexist with the >=3.14 helper-script environment.

suite2p 1.1.0 still reads the older output layouts this pipeline has
produced (ops/stat/F/Fneu/spks/iscell written with allow_pickle) and
treats an absent optional redcell.npy as "no channel 2" rather than an
error, verified through its own gui.io.load_files entry point, so
pinning the current release does not strand existing results.

Also harden findUv for the cross-platform case. MATLAB's system()
inherits a minimal PATH, so on unix probe `$SHELL -l -c` first, which
sources the user's profile and sees the real PATH; the previous plain
probe found nothing under that minimal PATH while the login shell
resolves /opt/homebrew/bin/uv. Extend the fallback install locations to
cover linuxbrew, /usr/bin and /snap/bin.

Fix an unrelated latent bug in OpenExtGUI while touching the launch
path: when system() threw, `out` was never assigned, so the following
`if out ~= 0` raised its own error and masked the real failure. Move
the exit-status check inside the try block.

The IBL atlas GUI (OpenExtGUI2) still uses the iblenv conda env and is
left alone here; migrating iblapps is a larger change.

Assisted-by: ClaudeCode:claude-opus-5
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant