Skip to content

fix: quote paths inside ChimeraX --cmd script to handle spaces#106

Merged
St3451 merged 2 commits into
masterfrom
fix/chimerax-path-quoting
May 28, 2026
Merged

fix: quote paths inside ChimeraX --cmd script to handle spaces#106
St3451 merged 2 commits into
masterfrom
fix/chimerax-path-quoting

Conversation

@St3451
Copy link
Copy Markdown
Collaborator

@St3451 St3451 commented May 28, 2026

Summary

PR #105 switched the ChimeraX invocation to an argv list + shell=False, which bypasses the OS shell. But path tokens were still inlined unquoted into the ChimeraX --cmd script, so ChimeraX's own parser would still split on whitespace. An output dir or build folder containing a space (e.g. user-chosen output paths) would silently break the open / save commands — no PNG, no Python-level error.

Wrap pdb_path, attr_file_path, and output_path in double quotes before interpolation so ChimeraX treats each as a single token. Paths with embedded double-quote characters are not supported.

Also tightened the _run_chimerax docstring so it accurately describes which parsing layer the argv-list change covers.

The previous argv-list refactor bypassed the OS shell but the path tokens
were still inlined unquoted into the ChimeraX --cmd script. ChimeraX's own
command parser still split on whitespace, so an output dir or build folder
containing a space (e.g. user-chosen output paths) would break "open" and
"save" commands silently — no PNG, no Python-level error.

Wrap pdb_path, attr_file_path, and output_path in double quotes before
interpolation so ChimeraX treats each as a single token. Paths with embedded
double quotes are not supported; build-datasets wouldn't accept them either.

Also tightened the _run_chimerax docstring to accurately describe what the
argv-list change does and doesn't cover.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 28, 2026 16:42
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes ChimeraX --cmd script parsing when file paths contain spaces by quoting the paths embedded inside the ChimeraX command string (separate from the OS-level argv parsing introduced in PR #105).

Changes:

  • Clarifies _run_chimerax() docstring to distinguish OS argv parsing (shell=False) from ChimeraX --cmd parsing.
  • Quotes pdb_path, attr_file_path, and output_path within the ChimeraX --cmd script so ChimeraX treats them as single tokens even with spaces.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/plotting/chimerax_plot.py Outdated
Comment thread scripts/plotting/chimerax_plot.py Outdated
Comment thread scripts/plotting/chimerax_plot.py
@St3451 St3451 merged commit 64f3f5e into master May 28, 2026
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.

2 participants