Standardize printing of MessageTransforms

[giorgossideris]

Why are these changes needed?

Through these changes better printing of the MessageTransforms stats is achieved. Before, printing was executed both in MessageTransform.apply_transform methods and the TransformMessages._transform_messages method (the last one incorrectly, as the required condition was always false as metioned in #2307). Also, there was no option to avoid these print statements.

This PR aims to fix all these problems, by including a print_stats method in the MessageTransform Protocol. In this way the printing is decoubled by the transforming. Also, to_print_stats argument has been added to TransformMessages constructor to indicate whether to print the transformations' stats or not.

Related issue number

Closes #2307.

Checks

• I've included any doc changes needed for https://microsoft.github.io/autogen/. See https://microsoft.github.io/autogen/docs/Contribute#documentation to build and test documentation locally.
• I've added tests (if relevant) corresponding to the changes introduced in this PR.
• I've made sure all auto checks have passed.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 24.24242% with 25 lines in your changes are missing coverage. Please review.

Project coverage is 15.54%. Comparing base (4a44093) to head (ffbb67b).
Report is 3 commits behind head on main.

Files	Patch %	Lines
...togen/agentchat/contrib/capabilities/transforms.py	31.57%	13 Missing ⚠️
...entchat/contrib/capabilities/transform_messages.py	14.28%	12 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2308       +/-   ##
===========================================
- Coverage   38.14%   15.54%   -22.60%     
===========================================
  Files          78       78               
  Lines        7865     7880       +15     
  Branches     1683     1824      +141     
===========================================
- Hits         3000     1225     -1775     
- Misses       4615     6627     +2012     
+ Partials      250       28      -222     

Flag	Coverage Δ
unittest	14.20% <24.24%> (?)
unittests	14.50% <0.00%> (-23.63%)	⬇️

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.

[giorgossideris]

@microsoft-github-policy-service agree

[WaelKarkoub]

@giorgossideris thank you for the PR! I have one question regarding the print_stats method you added to the protocol. How do you see this method being used for transformations such as rotating or scaling images, trimming audio files, or converting from one message type to another? These types of transformations may not necessarily involve statistics, but more so logging information.

[giorgossideris]

@giorgossideris thank you for the PR! I have one question regarding the print_stats method you added to the protocol. How do you see this method being used for transformations such as rotating or scaling images, trimming audio files, or converting from one message type to another? These types of transformations may not necessarily involve statistics, but more so logging information.

This is a valid question and maybe it is a good idea, to extend print_stats and make it a function of communicating useful information to the user (by printing, logging, saving to files etc.). Something like trace_metadata.

[WaelKarkoub]

The first iteration of this feature could stay simple; the method should return a string to print to the console for example. I'm only saying this because I can't anticipate the users' behavior/needs.

[giorgossideris]

The first iteration of this feature could stay simple;

Yes, your points are reasonable and I agree that we should probably keep it simple at the beginning.

the method should return a string to print to the console for example. I'm only saying this because I can't anticipate the users' behavior/needs.

About returning a string, I was thinking that since the string is meant for printing, it would better to be printed inside the function. Why do you think that returning a string is a better option?

[WaelKarkoub]

1. Makes it easier to test
2. I'm not sure what is the default way of printing a message in AutoGen (regular print, IOStream, logging). Seems like IOStream is the new default for printing. Instead of worrying about the user using the wrong print method, we decide for them instead

To add to (2), formatting the print statement will stay consistent across different transforms

[giorgossideris]

1. Makes it easier to test
2. I'm not sure what is the default way of printing a message in AutoGen (regular print, IOStream, logging). Seems like IOStream is the new default for printing. Instead of worrying about the user using the wrong print method, we decide for him instead

To add to (2), formatting the print statement will stay consistent across different transforms

Sounds good, I will make this change.

[WaelKarkoub]

Just an initial review before you implement the discussed changes

[WaelKarkoub]

@giorgossideris I apologize for the delay, I got pretty busy this week. We added a few tests in transform messages related to the token limiter that needs to be moved to the new file you created. #2364

@giorgossideris thank you for the PR! I have one question regarding the print_stats method you added to the protocol. How do you see this method being used for transformations such as rotating or scaling images, trimming audio files, or converting from one message type to another? These types of transformations may not necessarily involve statistics, but more so logging information.

This is a valid question and maybe it is a good idea, to extend print_stats and make it a function of communicating useful information to the user (by printing, logging, saving to files etc.). Something like trace_metadata.

I'm still not convinced about naming the new method get_stats, as a lot of the transformations won't report stats. We can name it get_logs to be more generic and let the implementor of the protocol decide what logs mean, as always I'm always open to other ideas. Other than that, this is a great PR, and will approve it as soon as the above changes are clarified.

[giorgossideris]

@giorgossideris I apologize for the delay, I got pretty busy this week. We added a few tests in transform messages related to the token limiter that needs to be moved to the new file you created. #2364

@giorgossideris thank you for the PR! I have one question regarding the print_stats method you added to the protocol. How do you see this method being used for transformations such as rotating or scaling images, trimming audio files, or converting from one message type to another? These types of transformations may not necessarily involve statistics, but more so logging information.

This is a valid question and maybe it is a good idea, to extend print_stats and make it a function of communicating useful information to the user (by printing, logging, saving to files etc.). Something like trace_metadata.

I'm still not convinced about naming the new method get_stats, as a lot of the transformations won't report stats. We can name it get_logs to be more generic and let the implementor of the protocol decide what logs mean, as always I'm always open to other ideas. Other than that, this is a great PR, and will approve it as soon as the above changes are clarified.

Thank you very much @WaelKarkoub, I made the changes, let me know if there is anything to add!

[WaelKarkoub]

Looks good, great PR!