Skip to content

fix: improve repair instruction #1309

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.

Sign up for GitHub

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

Merged
akihikokuroda merged 2 commits into from
Jun 22, 2026
+4 −1
Merged

fix: improve repair instruction #1309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ba4cab
improve repair instruction
akihikokuroda Jun 19, 2026
1e51a88
review comment
akihikokuroda Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion mellea/stdlib/sampling/feedback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,8 @@ def format_matplotlib_error(validation_result: ValidationResult) -> str:
)
return (
f"Your code didn't save the plot to the expected file. "
f"Try: Add plt.savefig('{expected_path}') before plt.show() or plt.close()."
f"Try: Add plt.savefig('{expected_path}') before plt.show() or plt.close(). "
f"Do not use a variable as an argument of plt.savefig. Use a string literal as the argument like plt.savefig('{expected_path}'). "

@AngeloDanducci AngeloDanducci Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

May be worth updating these two tests,
test_matplotlib_plot_file_saved_error_formatting
test_matplotlib_plot_file_saved_error_with_custom_path
to include: assert "string literal" in formatted or assert "Do not use a variable as an argument" in formatted.

Adding the assert would act as a safeguard if this gets changed in the future and cause a test failure/update required in case it is overlooked in a future update.

Otherwise LGTM!

@akihikokuroda akihikokuroda Jun 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks! I'll fix the tests..

)
else:
return (
Expand Down
2 changes: 2 additions & 0 deletions test/stdlib/sampling/test_presets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ def test_matplotlib_plot_file_saved_error_formatting(self):
assert "plot.png" in formatted
assert "plt.savefig('plot.png')" in formatted
assert "Try:" in formatted
assert "Do not use a variable as an argument" in formatted

def test_matplotlib_plot_file_saved_error_with_custom_path(self):
"""Test PlotFileSaved error formatting with custom output path."""
Expand All @@ -290,6 +291,7 @@ def test_matplotlib_plot_file_saved_error_with_custom_path(self):
formatted = ModelFriendlyFeedbackFormatter.format_matplotlib_error(result)
assert "/tmp/output.png" in formatted
assert "plt.savefig('/tmp/output.png')" in formatted
assert "Do not use a variable as an argument" in formatted

def test_matplotlib_plot_file_saved_error_fallback(self):
"""Test PlotFileSaved error formatting with malformed reason (fallback)."""
Expand Down
Loading