Skip to content

[Train] Allow contextlib.redirect_stdout() to bypass print redirect to logs#61075

Merged
matthewdeng merged 6 commits into
ray-project:masterfrom
Mr-Neutr0n:fix-print-patch-redirect
May 12, 2026
Merged

[Train] Allow contextlib.redirect_stdout() to bypass print redirect to logs#61075
matthewdeng merged 6 commits into
ray-project:masterfrom
Mr-Neutr0n:fix-print-patch-redirect

Conversation

@Mr-Neutr0n
Copy link
Copy Markdown
Contributor

When Ray Train's print patch is active, any library that uses contextlib.redirect_stdout() to capture print output gets broken — the patched redirected_print sends everything to the logger unconditionally when file is stdout or None, even if stdout has been replaced by a StringIO via redirect_stdout().

This is what happens with smart_open, which builds its docstring by capturing print output at import time. On Ray workers the print patch runs first, so when smart_open gets imported later as a transitive dep, the docstring text ends up in the logger instead of being captured.

The fix checks whether sys.stdout/sys.stderr have been redirected (i.e. they're not sys.__stdout__/sys.__stderr__) and falls back to the original print in that case. This way redirect_stdout() works as expected while normal prints still get routed through the logger.

Also fixed a minor bug on the custom-file fallback path where _original_print(objects, ...) was missing the * unpack (should be _original_print(*objects, ...)).

Fixes #61050

redirect_stdout() replaces sys.stdout with a StringIO, but
redirected_print was unconditionally sending to the logger when
file was stdout or None. now it checks whether stdout/stderr have
been redirected and falls back to the original print if so.

also fixes _original_print(objects, ...) -> _original_print(*objects, ...)
which was wrapping args in a tuple on the custom-file path.

Fixes ray-project#61050
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly fixes an issue where the patched print function in Ray Train would interfere with contextlib.redirect_stdout(). The fix involves checking if sys.stdout or sys.stderr have been redirected and, if so, falling back to the original print function. A new test case has been added to verify this behavior. Additionally, a minor bug related to argument unpacking in the call to the original print function is also fixed. The changes are logical and well-implemented. I've added one suggestion to make the code slightly more concise.

Comment thread python/ray/train/v2/_internal/logging/patch_print.py Outdated
@ray-gardener ray-gardener Bot added the community-contribution Contributed by the community label Feb 15, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 2, 2026

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

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 github-actions Bot added the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Mar 2, 2026
@github-actions
Copy link
Copy Markdown

This pull request has been automatically closed because there has been no more activity in the 14 days
since being marked stale.

Please feel free to reopen or open a new pull request if you'd still like this to be addressed.

Again, you can always ask for help on our discussion forum or Ray's public slack channel.

Thanks again for your contribution!

@github-actions github-actions Bot closed this Mar 16, 2026
@matthewdeng matthewdeng reopened this May 7, 2026
Signed-off-by: matthewdeng <matt@anyscale.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit a10bcb2. Configure here.

Comment thread python/ray/train/v2/_internal/logging/patch_print.py Outdated
Copy link
Copy Markdown
Member

@pseudo-rnd-thoughts pseudo-rnd-thoughts left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, overall looks good. Could you just merge the two if statements and add to the test

Comment thread python/ray/train/v2/_internal/logging/patch_print.py Outdated
Comment thread python/ray/train/v2/tests/test_logging.py Outdated
@github-actions github-actions Bot added unstale A PR that has been marked unstale. It will not get marked stale again if this label is on it. and removed stale The issue is stale. It will be closed within 7 days unless there are further conversation labels May 8, 2026
Mark Towers added 2 commits May 12, 2026 15:22
@pseudo-rnd-thoughts pseudo-rnd-thoughts added the go add ONLY when ready to merge, run all tests label May 12, 2026
@pseudo-rnd-thoughts pseudo-rnd-thoughts changed the title fix print patch breaking contextlib.redirect_stdout() [Train] Allow contextlib.redirect_stdout() to bypass print redirect to logs May 12, 2026
Signed-off-by: Mark Towers <mark.m.towers@gmail.com>
Signed-off-by: Mark Towers <mark.m.towers@gmail.com>
@matthewdeng matthewdeng merged commit 5810bb7 into ray-project:master May 12, 2026
5 checks passed
dancingactor pushed a commit to dancingactor/ray that referenced this pull request May 13, 2026
… to logs (ray-project#61075)

When Ray Train's print patch is active, any library that uses
`contextlib.redirect_stdout()` to capture print output gets broken — the
patched `redirected_print` sends everything to the logger
unconditionally when `file` is `stdout` or `None`, even if stdout has
been replaced by a `StringIO` via `redirect_stdout()`.

This is what happens with `smart_open`, which builds its docstring by
capturing print output at import time. On Ray workers the print patch
runs first, so when smart_open gets imported later as a transitive dep,
the docstring text ends up in the logger instead of being captured.

The fix checks whether `sys.stdout`/`sys.stderr` have been redirected
(i.e. they're not `sys.__stdout__`/`sys.__stderr__`) and falls back to
the original print in that case. This way `redirect_stdout()` works as
expected while normal prints still get routed through the logger.

Also fixed a minor bug on the custom-file fallback path where
`_original_print(objects, ...)` was missing the `*` unpack (should be
`_original_print(*objects, ...)`).

Fixes ray-project#61050

---------

Signed-off-by: matthewdeng <matt@anyscale.com>
Signed-off-by: Mark Towers <mark@anyscale.com>
Signed-off-by: Mark Towers <mark.m.towers@gmail.com>
Co-authored-by: matthewdeng <matt@anyscale.com>
Co-authored-by: Mark Towers <mark@anyscale.com>
Co-authored-by: Mark Towers <mark.m.towers@gmail.com>
am-kinetica pushed a commit to kineticadb/ray that referenced this pull request May 14, 2026
… to logs (ray-project#61075)

When Ray Train's print patch is active, any library that uses
`contextlib.redirect_stdout()` to capture print output gets broken — the
patched `redirected_print` sends everything to the logger
unconditionally when `file` is `stdout` or `None`, even if stdout has
been replaced by a `StringIO` via `redirect_stdout()`.

This is what happens with `smart_open`, which builds its docstring by
capturing print output at import time. On Ray workers the print patch
runs first, so when smart_open gets imported later as a transitive dep,
the docstring text ends up in the logger instead of being captured.

The fix checks whether `sys.stdout`/`sys.stderr` have been redirected
(i.e. they're not `sys.__stdout__`/`sys.__stderr__`) and falls back to
the original print in that case. This way `redirect_stdout()` works as
expected while normal prints still get routed through the logger.

Also fixed a minor bug on the custom-file fallback path where
`_original_print(objects, ...)` was missing the `*` unpack (should be
`_original_print(*objects, ...)`).

Fixes ray-project#61050

---------

Signed-off-by: matthewdeng <matt@anyscale.com>
Signed-off-by: Mark Towers <mark@anyscale.com>
Signed-off-by: Mark Towers <mark.m.towers@gmail.com>
Co-authored-by: matthewdeng <matt@anyscale.com>
Co-authored-by: Mark Towers <mark@anyscale.com>
Co-authored-by: Mark Towers <mark.m.towers@gmail.com>
Signed-off-by: anindyam1969 <amukherjee@kinetica.com>
Lucas61000 pushed a commit to Lucas61000/ray that referenced this pull request May 15, 2026
… to logs (ray-project#61075)

When Ray Train's print patch is active, any library that uses
`contextlib.redirect_stdout()` to capture print output gets broken — the
patched `redirected_print` sends everything to the logger
unconditionally when `file` is `stdout` or `None`, even if stdout has
been replaced by a `StringIO` via `redirect_stdout()`.

This is what happens with `smart_open`, which builds its docstring by
capturing print output at import time. On Ray workers the print patch
runs first, so when smart_open gets imported later as a transitive dep,
the docstring text ends up in the logger instead of being captured.

The fix checks whether `sys.stdout`/`sys.stderr` have been redirected
(i.e. they're not `sys.__stdout__`/`sys.__stderr__`) and falls back to
the original print in that case. This way `redirect_stdout()` works as
expected while normal prints still get routed through the logger.

Also fixed a minor bug on the custom-file fallback path where
`_original_print(objects, ...)` was missing the `*` unpack (should be
`_original_print(*objects, ...)`).

Fixes ray-project#61050

---------

Signed-off-by: matthewdeng <matt@anyscale.com>
Signed-off-by: Mark Towers <mark@anyscale.com>
Signed-off-by: Mark Towers <mark.m.towers@gmail.com>
Co-authored-by: matthewdeng <matt@anyscale.com>
Co-authored-by: Mark Towers <mark@anyscale.com>
Co-authored-by: Mark Towers <mark.m.towers@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution Contributed by the community go add ONLY when ready to merge, run all tests unstale A PR that has been marked unstale. It will not get marked stale again if this label is on it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Train] patch_print_function() breaks smart_open's docstring capture

3 participants