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

Fix image print for auto feedback from code notebook #1389

Conversation

pmalarme
Copy link
Collaborator

Why are these changes needed?

It solves the display of the chart in notebook/agentchat_auto_feedback_from_code_execution.ipynb. I also added .notebook in the .gitignore to be able to have a folder to update the notebooks for testing without risking to commit them.

Checks

@pmalarme pmalarme changed the title Feature/fix-image-pring-for-auto-feedback-from-code Feature/fix-image-print-for-auto-feedback-from-code Jan 24, 2024
@pmalarme pmalarme changed the title Feature/fix-image-print-for-auto-feedback-from-code Fix image print for auto feedback from code notebook Jan 24, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jan 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5eaf712) 33.08% compared to head (0da7a90) 40.86%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1389      +/-   ##
==========================================
+ Coverage   33.08%   40.86%   +7.78%     
==========================================
  Files          42       42              
  Lines        5048     5048              
  Branches     1156     1223      +67     
==========================================
+ Hits         1670     2063     +393     
+ Misses       3250     2795     -455     
- Partials      128      190      +62     
Flag Coverage Δ
unittests 40.86% <ø> (+7.82%) ⬆️

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.

.gitignore Outdated Show resolved Hide resolved
@davorrunje
Copy link
Collaborator

Let me just understand the problem before I get on my laptop and run it myself, the issue was that the image was not displayed in notebook?

If so, wouldn't it be better to check if the file exists, load it from file and then display it? That way we know what is in the file is actually what we are looking into. Not saying the PR is wrong, but this solution would be more robust.

@pmalarme
Copy link
Collaborator Author

pmalarme commented Jan 29, 2024

Let me just understand the problem before I get on my laptop and run it myself, the issue was that the image was not displayed in notebook?

If so, wouldn't it be better to check if the file exists, load it from file and then display it? That way we know what is in the file is actually what we are looking into. Not saying the PR is wrong, but this solution would be more robust.

The image does not display in GitHub Codespace. You just ask explicitly to display it and now it works.

@davorrunje
Copy link
Collaborator

LGTM 👍

@sonichi sonichi added this pull request to the merge queue Feb 2, 2024
Merged via the queue into microsoft:main with commit 710862d Feb 2, 2024
23 checks passed
whiskyboy pushed a commit to whiskyboy/autogen that referenced this pull request Apr 17, 2024
* Add .notebook folder to gitignore

* Add the import of display (IPython) and call it to display the chart

* Remove the update of .gitignore
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.

5 participants