-
Notifications
You must be signed in to change notification settings - Fork 261
Fix/replace print with logging #200
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
base: main
Are you sure you want to change the base?
Fix/replace print with logging #200
Conversation
locross93
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this @manaspros! The migration to absl.logging looks useful and good to provide better logging control.
Requested changes:
Please remove changes to all files under deprecated/ paths. We are in the process of removing these deprecated components from the codebase, so making changes to them creates unnecessary churn.
Once these files are removed from the PR, I'm happy to approve!
|
@locross93 Check and let me know |
concordia/contrib/language_models/together/together_ai_model.py
Outdated
Show resolved
Hide resolved
Fixes google-deepmind#187 This PR replaces all print() calls in the concordia package with proper structured logging using absl.logging, which is already a project dependency. This provides: - Better control over log levels (info, debug, warning, error) - Timestamps and module context in log output - Ability to filter/suppress output programmatically - Consistent logging approach across the codebase Changes: - Added 'from absl import logging' imports where needed - Replaced print() with appropriate logging levels: - logging.info() for informational output - logging.debug() for verbose/debug output - logging.warning() for warnings - logging.error() for error conditions - Used %s formatting style for better performance - Preserved termcolor.colored() calls for colored output Files modified include: - Core engine files (sequential.py, simultaneous.py, etc.) - Game master components - Language model implementations - Utilities (profiler.py with 38 statements, helper_functions.py, etc.) - Simulation prefabs - All contrib and deprecated components Testing: - All 144 tests pass - Verified 0 print() statements remain in concordia/ package
Remove unnecessary absl.logging import that was conflicting with standard library logging module. This file uses logging.getLogger() which is not available in absl.logging.
Use alias 'absl_logging' for absl.logging to avoid conflict with concordia.typing.logging module which defines LoggingChannel types.
…odules" This reverts commit 485ddbd.
5b589d6 to
a17e192
Compare
|
Sorry for so many commits changes |
|
Should i also remove concordia/components/agents/depreciated? @locross93 |
|
Remove any changes you made to |
|
Now everything is done u can merge |
|
We got this error on our end preventing a merge: Can you make sure your code is up to date with the latest changes. |
Fixes #187
This PR replaces all print() calls in the concordia package with proper structured logging using absl.logging, which is already a project dependency.
Benefits
Changes
Files Modified
Testing
Total Impact