Skip to content

Logger: setters return old value, coerce all msg components to string before concatenating#3589

Merged
dlebauer merged 13 commits intoPecanProject:developfrom
infotroph:logger-oldvals
Aug 6, 2025
Merged

Logger: setters return old value, coerce all msg components to string before concatenating#3589
dlebauer merged 13 commits intoPecanProject:developfrom
infotroph:logger-oldvals

Conversation

@infotroph
Copy link
Member

Description

Setters return old value

logger.set*(op) now invisibly returns the value of op that was in effect before the call, for ease of undoing temporary changes by the same pattern you might know from op <- options(...); on.exit(options(op)). I found I was wanting this to keep state clean during testing.

More robust string conversion

Cases where the message is composed from objects of mixed type now convert each argument to a string before concatenating them.

Before:

> logger.info("dates were", as.Date(c("2024-01-01", "2025-01-23")))
2025-08-06 02:19:58.225157 INFO   [console] : 
   dates were 19723 20111 

After:

> logger.info("dates were", as.Date(c("2024-01-01", "2025-01-23")))'
2025-08-06 02:09:12.461013 INFO   [console] : 
   dates were 2024-01-01, 2025-01-23

Also

  • whitespace cleanup
  • couple minor Roxygen wording tweaks

Motivation and Context

Review Time Estimate

  • Immediately
  • Within one week
  • When possible

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • My name is in the list of CITATION.cff
  • I agree that PEcAn Project may distribute my contribution under any or all of
    • the same license as the existing code,
    • and/or the BSD 3-clause license.
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@github-actions github-actions bot added the base label Aug 6, 2025
Copy link
Member

@dlebauer dlebauer left a comment

Choose a reason for hiding this comment

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

LGTM

TOOD: either merge documentation suggestions in infotroph#11 or let me know and I can point it to develop.

dlebauer and others added 9 commits August 6, 2025 13:52
Co-authored-by: Chris Black <chris@ckblack.org>
Co-authored-by: Chris Black <chris@ckblack.org>
Co-authored-by: Chris Black <chris@ckblack.org>
Co-authored-by: Chris Black <chris@ckblack.org>
Co-authored-by: Chris Black <chris@ckblack.org>
Ensure docs are complete and correct with respect to SEVERE vs ERROR
@github-actions github-actions bot added the tests label Aug 6, 2025
@infotroph infotroph requested a review from dlebauer August 6, 2025 22:38
Copy link
Member

@dlebauer dlebauer left a comment

Choose a reason for hiding this comment

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

👍🏻

@dlebauer dlebauer enabled auto-merge August 6, 2025 22:54
@dlebauer dlebauer added this pull request to the merge queue Aug 6, 2025
Merged via the queue into PecanProject:develop with commit 198010c Aug 6, 2025
18 of 25 checks passed
@infotroph infotroph deleted the logger-oldvals branch August 7, 2025 17:37
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.

2 participants