-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Adding headers for examples/units for MEP12/sphinx-gallery compliance #8254
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
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 some small changes to make, otherwise this all looks good 👍
============ | ||
Artist tests | ||
============ | ||
|
||
Test unit support with each of the matplotlib primitive artist types |
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.
Could you put a full stop at the end of this sentence?
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.
Also capitalize Matplotlib if you change this line.
============ | ||
Artist tests | ||
============ | ||
|
||
Test unit support with each of the matplotlib primitive artist types | ||
|
||
The axes handles unit conversions and the artists keep a pointer to |
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.
axes
--> axis
============ | ||
Artist tests | ||
============ | ||
|
||
Test unit support with each of the matplotlib primitive artist types | ||
|
||
The axes handles unit conversions and the artists keep a pointer to | ||
their axes parent, so you must init the artists with the axes instance |
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.
axes
--> axis
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.
I would change this 'init' to the full 'initialize'...
=================== | ||
Bar demo with units | ||
=================== | ||
|
||
plot using a variety of cm vs inches conversions. The example shows |
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.
plot
--> 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.
Also, spell out centimetre (inches is not abbreviated).
========================= | ||
|
||
This is the same example as | ||
<a href='http://matplotlib.org/examples/api/barchart_demo.html'> |
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.
I think there is probably a better way of linking between examples instead of using a hyperlink, but I can't think of how to at the moment.
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.
I don't think this works. Once we'll finish the migration to sphinx gallery, rst markup will work.
============ | ||
Radian ticks | ||
============ | ||
|
||
Plot with radians from the basic_units mockup example package |
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.
Could you add a full stop and a newline at the end of the first sentence?
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.
In line with @dstansby's comments, these are not exactly comments on what you've done, but general improvements, really. Depends on how much work you want to do...
============ | ||
Artist tests | ||
============ | ||
|
||
Test unit support with each of the matplotlib primitive artist types |
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.
Also capitalize Matplotlib if you change this line.
============ | ||
Artist tests | ||
============ | ||
|
||
Test unit support with each of the matplotlib primitive artist types | ||
|
||
The axes handles unit conversions and the artists keep a pointer to | ||
their axes parent, so you must init the artists with the axes instance |
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.
I would change this 'init' to the full 'initialize'...
============ | ||
Artist tests | ||
============ | ||
|
||
Test unit support with each of the matplotlib primitive artist types | ||
|
||
The axes handles unit conversions and the artists keep a pointer to | ||
their axes parent, so you must init the artists with the axes instance | ||
if you want to initialize them with unit data, or else they will not |
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.
... and 'initialize them with unit data' to 'use them with unit data'.
@@ -34,13 +38,15 @@ | |||
ax.add_collection(lc) | |||
|
|||
# test a plain-ol-line | |||
line = lines.Line2D([0*cm, 1.5*cm], [0*cm, 2.5*cm], lw=2, color='black', axes=ax) | |||
line = lines.Line2D( | |||
[0*cm, 1.5*cm], [0*cm, 2.5*cm], lw=2, color='black', axes=ax) |
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.
If this is too long, I'd break after the positional arguments instead; this doesn't look that great.
ax.add_line(line) | ||
|
||
if 0: | ||
# test a patch | ||
# Not supported at present. | ||
rect = patches.Rectangle((1*cm, 1*cm), width=5*cm, height=2*cm, alpha=0.2, axes=ax) | ||
rect = patches.Rectangle( | ||
(1*cm, 1*cm), width=5*cm, height=2*cm, alpha=0.2, axes=ax) |
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.
Same here.
=================== | ||
Bar demo with units | ||
=================== | ||
|
||
plot using a variety of cm vs inches conversions. The example shows |
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.
Also, spell out centimetre (inches is not abbreviated).
=================== | ||
Bar demo with units | ||
=================== | ||
|
||
plot using a variety of cm vs inches conversions. The example shows | ||
how default unit instrospection works (ax1), how various keywords can |
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.
instrospection -> introspection
@@ -1,8 +1,13 @@ | |||
""" | |||
========== | |||
Evans test |
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.
Who or what is "Evans"? Does anyone know? @tacaswell? I think this needs a different name.
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.
This all predates me. I assume someone from JPL
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.
LGTM
Thanks @yinleon !
Cool, going to merge this in to get the titles in ready for sphinx-gallery, and open a new PR with the suggested cleanups. Thanks for the PR @yinleon ! |
wohoo! This section did not appear in our gallery. I'm going to try to add to the sphinx-gallery one. |
Adding headers for examples/units for MEP12/sphinx-gallery compliance
Backported to |
Changes in accordance to #7206