-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Example: remove overlapping text from image_masked.py #7682
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
Conversation
This also makes several other changes that clarify the example and improve the code.
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.
Overall this looks good. I have two nitpicks, that I think would be nice to fix.
x = y = np.arange(-3.0, 3.0, delta) | ||
x0, x1 = -5, 5 | ||
y0, y1 = -3, 3 | ||
x = np.linspace(x0, x1, 500) |
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.
👍
@@ -38,24 +40,33 @@ | |||
# Anything above that range is colored based on palette.set_over, etc. | |||
|
|||
# set up the axes | |||
fig, (ax1, ax2) = plt.subplots(1, 2) | |||
fig, (ax1, ax2) = plt.subplots(2, 1, figsize=(6, 5.4)) |
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.
Isn't the second argument unecessary?
I also tend to favor being explicit : nrows=2
.
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.
Agreed. I will fix this.
ax1.set_title('Green=low, Red=high, Blue=masked') | ||
cbar = fig.colorbar(im, extend='both', shrink=0.9, ax=ax1) | ||
cbar.set_label('uniform') | ||
for ticklabel in ax1.xaxis.get_ticklabels(): |
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.
The x-axis could be shared, which renders these two lines unecessary.
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.
Actually, both axis could be shared.
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.
On the contrary, axis sharing is inconsistent with the default aspect-ratio handling of imshow. This is a fundamental limitation (to avoid an over-determined system), not a matter of implementation. Since imshow is typically used in cases where unit aspect ratio is desired, I decided to leave that default and forgo the axis sharing.
It would also be nice to use the new color scheme instead of the "r", "b", "g". |
Regarding colors: In this case, I think that using the simple primary colors, with their simple name abbreviations in the code, is preferable for the purposes of illustration. |
This also makes several other changes that clarify the example
and improve the code.