-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DOC: Added plt.show() at the end of each necessary block #46231
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
16cbbaa
to
77e6e25
Compare
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.
why are you changing to use plt.show()
and not just adding ;
which render the plots?
|
||
@savefig 04_airqual_quick.png | ||
air_quality.plot() | ||
In [1]: air_quality.plot() |
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.
umm you can just add a ;
no?
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.
Hi @jreback. Thanks for your comment. I understand that adding the semicolon at the end of air_quality.plot() line will suppress the Out[N]: AxesSubplot:xlabel='datetime' line from the output, but that's not how I understood the purpose of issue #45772.
I believe that the fix for the issue needs to explicitly show the plt.show(), not because it is necessary in that particular example, but so that the user understands plt.show() is the function that can be called to render the plot, rather than it just automagically appearing below the cell. Without this, a new user may believe that the call to plot() is actually rendering the figure. In the scenario below, for example, we need to explicitly call plt.show() to render the charts at the right point in the code. I think the hope is that including plt.show() in the example will provide them with the knowledge to be able to do this.
for station in air_quality.columns:
print(station)
fig = air_quality[station].plot(figsize=(12, 4))
fig.set_ylabel("NO$_2$ concentration")
plt.show()
fig = air_quality[station].plot.area(figsize=(12, 4))
fig.set_ylabel("NO$_2$ concentration")
plt.show()
bd9ba34
to
cb3fe2d
Compare
92c3cb2
to
cb426b5
Compare
|
||
@savefig 04_airqual_scatter.png | ||
air_quality.plot.scatter(x="station_london", y="station_paris", alpha=0.5) | ||
|
||
In [1]: air_quality.plot.scatter(x="station_london", y="station_paris", alpha=0.5) |
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.
why are you not letting these run with python mode. this is not what we do elsewhere.
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.
Hi @jreback, if I run them in Python mode it displays as below with the extra Out[4] line, which is why I didn't run them in Python mode. I just thought it looked cleaner that way as the Out line doesn't add value.
If we wanted to get rid of this out line while still running in Python mode I can add semi colons to the end of each .plot() line as demonstrated below. Which of these would you prefer me to do?
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.
just assign it instead. this is a bit non-standard what you are doing and that's not great.
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.
@jreback how does this look? Many thanks for your help
77bc42b
to
2dc4968
Compare
…45772) Reworded and moved title to the top of the page
2dc4968
to
4cdb735
Compare
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.
Sure, this change looks okay. LGTM
thanks @ehallam |
Added plt.show() at the end of each necessary block.
Moved and reworded title section.
Tests added and passed if fixing a bug or adding a new featureAll code checks passed.Added an entry in the latestdoc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.