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

Add Preformatted class for logging preformatted messages #2998

Merged
merged 4 commits into from
Sep 24, 2023

Conversation

jsiirola
Copy link
Member

Fixes # .

Summary/Motivation:

MindtPy and PyROS both want to emit solver iteration logs and do NOT want those messages to be reformatted or line-wrapped. Their current solution is to hijack the entire Pyomo logging system to suppress the automatic formatting that the default log handler implements. This PR simplifies that by defining a Preformatted class that users can wrap a preformatted log message in before sending it to the logger. These messages will then be directly emitted (without being processed by the default formatter). As Preformatted is castable to str, it is compatible with other user-defined log handlers / formatters (without requiring any specialized handling).

Changes proposed in this PR:

  • define a Preformatted wrapper class for logging preformatted messages
  • Update the default Pyomo formatter to ignore / do not reformat Preformatted messages.
  • Add tests

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

codecov bot commented Sep 18, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.22% 🎉

Comparison is base (c471729) 87.85% compared to head (7776828) 88.08%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2998      +/-   ##
==========================================
+ Coverage   87.85%   88.08%   +0.22%     
==========================================
  Files         769      769              
  Lines       89497    89507      +10     
==========================================
+ Hits        78632    78838     +206     
+ Misses      10865    10669     -196     
Flag Coverage Δ
linux 85.18% <100.00%> (+1.44%) ⬆️
osx 74.98% <100.00%> (+<0.01%) ⬆️
other 85.36% <100.00%> (+1.45%) ⬆️
win 82.42% <100.00%> (+0.04%) ⬆️

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

Files Changed Coverage Δ
pyomo/common/log.py 97.12% <100.00%> (+0.17%) ⬆️

... and 7 files with indirect coverage changes

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

Copy link
Contributor

@mrmundt mrmundt left a comment

Choose a reason for hiding this comment

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

Lovely and elegant solution. Minor grammar weirdness.

pyomo/common/tests/test_log.py Outdated Show resolved Hide resolved
pyomo/common/tests/test_log.py Outdated Show resolved Hide resolved
pyomo/common/tests/test_log.py Outdated Show resolved Hide resolved
@jsiirola jsiirola changed the title Add Preformatted class for logging preformatted messages Add Preformatted class for logging preformatted messages Sep 19, 2023
Copy link
Contributor

@shermanjasonaf shermanjasonaf left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@jsiirola jsiirola merged commit bdbcb31 into Pyomo:main Sep 24, 2023
30 checks passed
@jsiirola jsiirola deleted the preformatted-logger branch September 24, 2023 18:36
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.

3 participants