Skip to content

Resolve errors in TeeStream and capture_output#3497

Merged
blnicho merged 8 commits intoPyomo:mainfrom
jsiirola:capture_output_jupyter
Mar 5, 2025
Merged

Resolve errors in TeeStream and capture_output#3497
blnicho merged 8 commits intoPyomo:mainfrom
jsiirola:capture_output_jupyter

Conversation

@jsiirola
Copy link
Copy Markdown
Member

Fixes #3487, partially addresses #3484

Summary/Motivation:

The recent rework of Pyomo's TeeStream / capture_output functionality to better support highspy inadvertently broke the ability to run within Jupyter notebooks (see #3487). This PR reworks how we manage capturing file descriptors to both avoid the error reported in the issue and to ensure that all output generated within capture_output is actually captured and redirected to the intended stream.

Additionally, this PR ensures that errors generated within capture_output are not themselves suppressed by capture_output and instead are allowed to propagate out to the surrounding context. In particularly, this ensures that passing non-TextIOStream objects to capture_output actually generates (sensible) errors for the user:

>>> from pyomo.common.tee import capture_output
>>> import logging
>>> with capture_output(logging.getLogger()):
...     print("hi")
... 
Error writing to output stream <logging.RootLogger @ 0x7f369963fdd0>:
    AttributeError: 'RootLogger' object has no attribute 'write'
Is this a writeable TextIOBase object?
The following was left in the output buffer:
    'hi\n'

Changes proposed in this PR:

  • TeeStream shouldn't need to open new streams to underlying file descriptors
  • capture_output(capture_fd=True) should capture up to 4 file descriptors: 1, 2, sys.stdout.fileno(), and sys.stderr.fileno()
  • capture_output should NOT capture errors generated within pyomo.common.tee
  • Improve the errors generated by capture_output, including the output of the exception that triggered the error.

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 28, 2025

Codecov Report

Attention: Patch coverage is 95.65217% with 2 lines in your changes missing coverage. Please review.

Project coverage is 88.43%. Comparing base (ab59eb2) to head (ffa2571).

Files with missing lines Patch % Lines
pyomo/common/tee.py 95.65% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3497      +/-   ##
==========================================
- Coverage   88.64%   88.43%   -0.21%     
==========================================
  Files         881      881              
  Lines      100873   100896      +23     
==========================================
- Hits        89421    89230     -191     
- Misses      11452    11666     +214     
Flag Coverage Δ
linux 86.08% <95.65%> (+<0.01%) ⬆️
osx 76.11% <95.65%> (+<0.01%) ⬆️
other 86.08% <95.65%> (-0.53%) ⬇️
win ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@blnicho blnicho requested review from blnicho and mrmundt March 4, 2025 19:04
@blnicho blnicho merged commit 8883d75 into Pyomo:main Mar 5, 2025
33 of 35 checks passed
@jsiirola jsiirola deleted the capture_output_jupyter branch March 24, 2025 19:51
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.

Pyomo 6.9.0 TeeStream broken in Jupyter

3 participants