Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Fix regression with run_subprocess #11230

Merged
merged 4 commits into from
Oct 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 14 additions & 12 deletions mne/utils/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,21 +138,22 @@ def run_subprocess(command, return_code=False, verbose=None, *args, **kwargs):
except Empty:
break
else:
out = out.decode('utf-8')
# Strip newline at end of the string, otherwise we'll end
# up with two subsequent newlines (as the logger adds one)
#
# XXX Once we drop support for Python <3.9, uncomment the
# following line and remove the if/else block below.
#
# out = out.decode('utf-8').removesuffix('\n')
out = out.decode('utf-8')
# log_out = out.removesuffix('\n')
if sys.version_info[:2] >= (3, 9):
out = out.removesuffix('\n')
log_out = out.removesuffix('\n')
elif out.endswith('\n'):
log_out = out[:-1]
else:
if out.endswith('\n'):
out = out[:-1]
log_out = out

logger.info(out)
logger.info(log_out)
all_out += out

while True: # process stderr
Expand All @@ -161,27 +162,28 @@ def run_subprocess(command, return_code=False, verbose=None, *args, **kwargs):
except Empty:
break
else:
err = err.decode('utf-8')
# Strip newline at end of the string, otherwise we'll end
# up with two subsequent newlines (as the logger adds one)
#
# XXX Once we drop support for Python <3.9, uncomment the
# following line and remove the if/else block below.
#
# err = err.decode('utf-8').removesuffix('\n')
err = err.decode('utf-8')
# err_out = err.removesuffix('\n')
if sys.version_info[:2] >= (3, 9):
err = err.removesuffix('\n')
err_out = err.removesuffix('\n')
elif err.endswith('\n'):
err_out = err[:-1]
else:
if err.endswith('\n'):
err = err[:-1]
err_out = err

# Leave this as logger.warning rather than warn(...) to
# mirror the logger.info above for stdout. This function
# is basically just a version of subprocess.call, and
# shouldn't emit Python warnings due to stderr outputs
# (the calling function can check for stderr output and
# emit a warning if it wants).
logger.warning(err)
logger.warning(err_out)
all_err += err

if do_break:
Expand Down
35 changes: 34 additions & 1 deletion mne/utils/tests/test_misc.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import os
import sys

import pytest

import mne
from mne.utils import sizeof_fmt
from mne.utils import sizeof_fmt, run_subprocess, catch_logging


def test_sizeof_fmt():
Expand Down Expand Up @@ -29,3 +32,33 @@ def test_html_repr():
del os.environ[key]
if existing_value is not None:
os.environ[key, existing_value]


@pytest.mark.parametrize('kind', ('stdout', 'stderr'))
def test_run_subprocess(tmp_path, kind):
"""Test run_subprocess."""
fname = tmp_path / 'subp.py'
with open(fname, 'w') as fid:
fid.write(f"""\
import sys
print('foo', file=sys.{kind})
print('bar', file=sys.{kind})
""")
with catch_logging() as log:
stdout, stderr = run_subprocess(
[sys.executable, str(fname)], verbose=True)
log = log.getvalue()
log = '\n'.join(log.split('\n')[1:]) # get rid of header
log = log.replace('\r\n', '\n') # Windows
stdout = stdout.replace('\r\n', '\n')
stderr = stderr.replace('\r\n', '\n')
want = 'foo\nbar\n'
assert log == want
if kind == 'stdout':
std = stdout
other = stderr
else:
std = stderr
other = stdout
assert std == want
assert other == ''