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

remove unsupported end from ami logs and remove MultilineLogger #8550

Closed
wants to merge 0 commits into from

Conversation

braingram
Copy link
Collaborator

@braingram braingram commented Jun 11, 2024

jwst.associations.lib.log_config globally sets the logging class to MultilineLogger which can cause seemingly unrelated tests to fail: https://github.com/spacetelescope/stpipe/actions/runs/9445531594/job/26087417184#step:10:189

The failure revealed a bug in the ami log messages (end is not a supported kwarg for these calls).

This PR also removes MultilineLogger and the global call to setLoggerClass.

This class was added in the "inital commit":
5915b0e#diff-9e38d1ac7b11ff469184d069a47776082c49c376e91e3b8dbb9e3b3872a6627bR177
and will change other logging if the association module is imported. For example:

import jwst.associations.lib.log_config
import logging

logger = logging.getLogger("foo")
logger.warning("bam\nbar")

produces:

2024-06-15 12:58:48,873 - stpipe - WARNING - bam
2024-06-15 12:58:48,873 - stpipe - WARNING - bar

removing import jwst.associations.lib.log_config produces:

bam
bar

(note the rather extensive log formatting that stpipe introduces, this is not part of this PR).
With this PR the output (with the jwst.association.lib.log_config) is:

2024-06-15 13:00:52,386 - stpipe - WARNING - bam
bar

(and the same if jwst.associations.lib.log_config is not imported).

Fixes #6407
and is an alternative to #7616

Regression tests running at: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1534/

Checklist for PR authors (skip items if you don't have permissions or they are not applicable)

  • added entry in CHANGES.rst within the relevant release section
  • updated or added relevant tests
  • updated relevant documentation
  • added relevant milestone
  • added relevant label(s)
  • ran regression tests, post a link to the Jenkins job below.
    How to run regression tests on a PR
  • All comments are resolved
  • Make sure the JIRA ticket is resolved properly

Copy link

codecov bot commented Jun 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.63%. Comparing base (dba65ae) to head (a7deded).
Report is 223 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8550   +/-   ##
=======================================
  Coverage   58.63%   58.63%           
=======================================
  Files         391      391           
  Lines       39081    39081           
=======================================
  Hits        22914    22914           
  Misses      16167    16167           

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

@braingram braingram force-pushed the multiline_logger branch 2 times, most recently from d9b5755 to 6ec6095 Compare June 15, 2024 16:57
@github-actions github-actions bot added the ami label Jun 15, 2024
@braingram braingram changed the title remove MultilineLogger remove unsupported end from ami logs and remove MultilineLogger Jun 15, 2024
@braingram braingram force-pushed the multiline_logger branch 3 times, most recently from 7ff2437 to a7deded Compare June 17, 2024 15:23
@braingram braingram closed this Jun 17, 2024
@braingram braingram deleted the multiline_logger branch August 7, 2024 17:28
@braingram braingram restored the multiline_logger branch August 7, 2024 17:29
@braingram braingram reopened this Aug 7, 2024
@braingram braingram closed this Aug 7, 2024
@braingram braingram deleted the multiline_logger branch August 7, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Log raises TypeError when matplotlib usetex=True
1 participant