Repository navigation
[Tune] Sanitize Windows-reserved characters in trial directory names - #65286
Merged
matthewdeng merged 3 commits intoAug 31, 2026
Merged
matthewdeng merged 3 commits into
matthewdeng merged 3 commits into
Conversation
Trial._generate_dirname() only stripped '/', '(' and ')' before using
the result as a directory name. On Windows, a colon (or any of
<>:"|?*) in the name raises NotADirectoryError (WinError 267) when
the trial tries to create its own directory, e.g. an env id like
"ale_py:ALE/Pong-v5" produces "PPO_ale_py:ALE-Pong-v5_...".
Extend the existing regex to also strip the full set of Windows
reserved filename characters. This is applied on every platform (not
gated on os.name), matching the existing '/', '(', ')' stripping that
is already unconditional for rsync compatibility, so a trial directory
created on Linux stays usable if later copied to Windows.
Fixes ray-project#49477
Signed-off-by: stantheman0128 <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates Trial._generate_dirname in Ray Tune to strip Windows-reserved filename characters (such as angle brackets, colons, double quotes, backslashes, pipes, question marks, and asterisks) on all platforms, ensuring that trial directory names remain valid across different operating systems and environments. Additionally, new unit tests have been added to verify that both generated and custom directory names are properly sanitized. There are no review comments, so no further feedback is provided.
ray-gardener
Bot
added
tune
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
github-actions
Bot
added
the
stale
github-actions
Bot
added
unstale
pseudo-rnd-thoughts
approved these changes
Aug 28, 2026
pseudo-rnd-thoughts
left a comment
pseudo-rnd-thoughts
left a comment
Member
There was a problem hiding this comment.
Thanks for the PR, LGTM
nadongjun
pushed a commit
to nadongjun/ray
that referenced
this pull request
Oct 6, 2026
…ay-project#65286) ## Description `Trial._generate_dirname()` only stripped `/`, `(`, and `)` before using the result as a directory name. On Windows, a colon or any of `<>:"|?*` in the generated name raises `NotADirectoryError` (WinError 267) when the trial tries to create its own directory. An env id such as `ale_py:ALE/Pong-v5` produces a dirname that still contains a colon after the existing sanitization, and Windows refuses to create it. This extends the regex to strip the full set of Windows-reserved filename characters. The stripping stays unconditional across platforms, matching how `/`, `(`, `)` are already stripped regardless of OS for rsync compatibility. That way a directory created on Linux stays usable if the results are later copied to a Windows machine. Reproduced on Windows 11 with the unmodified class: a trial whose generated name contains a colon fails with `NotADirectoryError: [WinError 267] The directory name is invalid` when `Trial.init_local_path()` calls `os.makedirs`. After the fix the same trial produces a sanitized name and the directory is created without error. ## Related issues Fixes ray-project#49477 ## Additional information Added `test_generate_dirname_strips_reserved_characters` and a parametrized `test_generate_dirname_custom_dirname_is_sanitized` to `python/ray/tune/tests/test_trial.py`, covering the reserved characters directly. Verified against the pre-fix code that both tests fail with the reported error, and pass after the fix. Formatted with `black` (repo-pinned version) and checked with `ruff`. Signed-off-by: stantheman0128 <[email protected]> Co-authored-by: Mark Towers <[email protected]>
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.
Description
Trial._generate_dirname()only stripped/,(, and)before using the result as a directory name. On Windows, a colon or any of<>:"|?*in the generated name raisesNotADirectoryError(WinError 267) when the trial tries to create its own directory. An env id such asale_py:ALE/Pong-v5produces a dirname that still contains a colon after the existing sanitization, and Windows refuses to create it.This extends the regex to strip the full set of Windows-reserved filename characters. The stripping stays unconditional across platforms, matching how
/,(,)are already stripped regardless of OS for rsync compatibility. That way a directory created on Linux stays usable if the results are later copied to a Windows machine.Reproduced on Windows 11 with the unmodified class: a trial whose generated name contains a colon fails with
NotADirectoryError: [WinError 267] The directory name is invalidwhenTrial.init_local_path()callsos.makedirs. After the fix the same trial produces a sanitized name and the directory is created without error.Related issues
Fixes #49477
Additional information
Added
test_generate_dirname_strips_reserved_charactersand a parametrizedtest_generate_dirname_custom_dirname_is_sanitizedtopython/ray/tune/tests/test_trial.py, covering the reserved characters directly. Verified against the pre-fix code that both tests fail with the reported error, and pass after the fix. Formatted withblack(repo-pinned version) and checked withruff.