fix: address PR feedback to improve Matplotlib side-by-side display#4
Conversation
Updates the plotting in `ex_2` of `ipc_s01e01.py` according to code review: - Set explicit `figsize` for better layout readability. - Removed axis ticks using `plt.xticks([]), plt.yticks([])` to focus entirely on the image content. - Added `plt.tight_layout()` to ensure titles don't overlap. Co-authored-by: Michal-Fularz <3768498+Michal-Fularz@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the image processing script to correctly display images in Matplotlib by converting from BGR to RGB and providing a side-by-side comparison. The review feedback recommends using plt.axis('off') instead of chaining xticks and yticks calls for better adherence to PEP 8 and idiomatic Matplotlib usage.
| plt.subplot(1, 2, 1) | ||
| plt.imshow(img_color) | ||
| plt.title('BGR (Incorrect)') | ||
| plt.xticks([]), plt.yticks([]) |
There was a problem hiding this comment.
Chaining multiple function calls on a single line using a comma is non-idiomatic in Python and violates PEP 8's recommendation against multiple statements on the same line. For image visualization in Matplotlib, it is more idiomatic to use plt.axis('off') to hide both the axes and the ticks.
| plt.xticks([]), plt.yticks([]) | |
| plt.axis('off') |
References
- PEP 8 discourages multiple statements on the same line. Chaining calls with commas creates an implicit tuple and is considered poor style. (link)
| plt.subplot(1, 2, 2) | ||
| plt.imshow(img_rgb) | ||
| plt.title('RGB (Correct)') | ||
| plt.xticks([]), plt.yticks([]) |
There was a problem hiding this comment.
As with the previous subplot, consider using plt.axis('off') instead of chaining xticks and yticks calls. This adheres to PEP 8 and follows standard Matplotlib practices for displaying images.
| plt.xticks([]), plt.yticks([]) | |
| plt.axis('off') |
References
- PEP 8 discourages multiple statements on the same line. (link)
Updates the plotting in `ex_2` of `ipc_s01e01.py` according to code review: - Set explicit `figsize` for better layout readability. - Removed axis ticks using `plt.xticks([]), plt.yticks([])` to focus entirely on the image content. - Added `plt.tight_layout()` to ensure titles don't overlap. Co-authored-by: Michal-Fularz <3768498+Michal-Fularz@users.noreply.github.com>
Updates the plotting in `ex_2` of `ipc_s01e01.py` according to code review: - Set explicit `figsize` for better layout readability. - Removed axis ticks using `plt.xticks([]), plt.yticks([])` to focus entirely on the image content. - Added `plt.tight_layout()` to ensure titles don't overlap. Co-authored-by: Michal-Fularz <3768498+Michal-Fularz@users.noreply.github.com>
1 participant
Applied code review feedback for the Matplotlib side-by-side view, setting figsize, removing ticks, and using tight_layout.
PR created automatically by Jules for task 13488927482085545876 started by @Michal-Fularz