Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
changes for MEP12/sphinx-gallery compliance#8209
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| @@ -1,4 +1,8 @@ | |||
| """ | |||
| =========== | |||
| Poly Editor | |||
| ========== | |||
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.
missing '='
examples/event_handling/resample.py Outdated
| # A class that will downsample the data and recompute when zoomed. | ||
| class DataDisplayDownsampler(object): | ||
| def __init__(self, xdata, ydata): | ||
| self.origYData = ydata | ||
| self.origXData = xdata | ||
| self.ratio =5 | ||
| self.ratio =50 |
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 seems a bit too aggressive.
Left 2 small comments, but otherwise looks good! |
In the future, it is better to make PRs against some branchother than your master branch. |
@tacaswell will do! My first commit to an OSS project, so apologies. |
| ax.axis[direction].set_visible(True) | ||
| for direction in ["left", "right", "bottom", "top"]: | ||
| # hides boarders |
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.
boarders -> borders
examples/event_handling/pipong.py Outdated
| A matplotlib based game of Pong illustrating one way to write interactive animation which are easily ported to multiple backends pipong.py was written by Paul Ivanov <http://pirsquared.org> |
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.
matplotlib -> Matplotlib
animation -> animations
Add period at end of sentence.
Also, please wrap this line at 79 characters.
examples/event_handling/resample.py Outdated
| Resampling Data | ||
| =============== | ||
| Downsampling lowers the sample rate or sample size of a signal. In this tutorial, the signal is downsampled when the plot is adjusted through dragging and zooming. |
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.
Wrap this line.
| Demonstrate the mixing of 2d and 3d subplots. | ||
| ============================================ | ||
| Demonstrate the mixing of 2d and 3d subplots | ||
| ============================================ |
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.
Missing blank line after, I think.
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 I resolved this?
| ================= | ||
| This is an example of creating a stacked bar plot with error bars using `plt.bar`. | ||
| Note the parameters `yerr` used for error bars, and `bottom` to stack the women's bars on top of the men's bars. |
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.
Lines need to be wrapped.
examples/pylab_examples/dolphin.py Outdated
| @@ -1,3 +1,11 @@ | |||
| """ | |||
| ======= | |||
| Dophins | |||
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.
Dophins -> Dolphins
examples/pylab_examples/dolphin.py Outdated
| Dophins | ||
| ======= | ||
| This example shows how to draw, and manipulate shapes given verticies and nodes using the patches, path and transforms classes. |
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.
Wrap line.
| Filling the area between lines | ||
| ============================== | ||
| This example shows how to use fill_between() to color between lines based on user-defined logic. |
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.
Wrap line.
examples/pylab_examples/geo_demo.py Outdated
| ====================== | ||
| This shows 4 possible projections using subplot. | ||
| Matplotlib also supports the <a href='http://matplotlib.org/basemap/'>Basemaps Toolkit</a> for geographic projections. |
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.
Wrap line. Probably should mention Cartopy as well.
examples/pylab_examples/mri_demo.py Outdated
| MRI | ||
| === | ||
| This example illustrates how to read an image (of an MRI) into a numpy array, and display it in greyscale using `ax.imshow`. |
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.
numpy -> NumPy
Wrap line.
| ax.axis[direction].set_visible(True) | ||
| for direction in ["left", "right", "bottom", "top"]: | ||
| # hides boarders |
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.
borders
examples/event_handling/pipong.py Outdated
| # pipong.py was written by Paul Ivanov <http://pirsquared.org> | ||
| """ | ||
| ==== | ||
| PONG |
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.
"Pong" (I don't think it should be all caps).
examples/pylab_examples/dolphin.py Outdated
| @@ -1,3 +1,11 @@ | |||
| """ | |||
| ======= | |||
| Dophins | |||
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.
dolphins
| @@ -1,3 +1,10 @@ | |||
| """ | |||
| ================ | |||
| Axis Line Styles | |||
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.
Looks like most already converted examples only capitalize the first word (which I prefer too), so let's stay consistent (applies to this and all files below).
| Demonstrate the mixing of 2d and 3d subplots. | ||
| ============================================ | ||
| Demonstrate the mixing of 2d and 3d subplots | ||
| ============================================ |
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.
blank line
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 I resolved this?
examples/event_handling/pipong.py Outdated
| A matplotlib based game of Pong illustrating one way to write interactive animation which are easily ported to multiple backends pipong.py was written by Paul Ivanov <http://pirsquared.org> |
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.
Here and below, wrap your lines at 79 characters (most editors should provide some option to help that).
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.
Here and below, use the rst syntax for hyperlinks (http://www.sphinx-doc.org/en/stable/rest.html#hyperlinks).
| Filling the area between lines | ||
| ============================== | ||
| This example shows how to use fill_between() to color between lines based on user-defined logic. |
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.
`fill_between`(with backquotes) -- ultimately this should create a link to the API docs.
examples/pylab_examples/dolphin.py Outdated
| Dophins | ||
| ======= | ||
| This example shows how to draw, and manipulate shapes given verticies and nodes using the patches, path and transforms classes. |
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.
"vertices".
Please spell-check your docstrings.
examples/pylab_examples/mri_demo.py Outdated
| MRI | ||
| === | ||
| This example illustrates how to read an image (of an MRI) into a numpy array, and display it in greyscale using `ax.imshow`. |
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.
justimshow
Apparently I reviewed this at the same time as@QuLogic :-) |
Apparently other reviewer can spell better than I can.. |
examples/event_handling/pipong.py Outdated
| A Matplotlib based game of Pong illustrating one way to write interactive | ||
| animations which are easily ported to multiple backends pipong.py was written | ||
| by <a href='http://pirsquared.org'>Paul Ivanov</a>. |
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 am somewhat not surprised by the original author of that example…
ping@ivanov :D
should I be alarmed about the Travis CLI failure? Now sure I understand what I'm seeing under Details. |
examples/event_handling/pipong.py Outdated
| Pong | ||
| ==== | ||
| A Matplotlib based game of Pong illustrating one way to write interactive |
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.
Beware of the trailing whitespace (remove it for PEP8-compliance)
examples/event_handling/pipong.py Outdated
| ==== | ||
| A Matplotlib based game of Pong illustrating one way to write interactive | ||
| animations which are easily ported to multiple backends pipong.py was written |
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.
As above, just remove the trailing whitespace for PEP8-compliance
afvincent commentedMar 7, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The Travis failure is related to PEP8 (more precisely to two trailing whitespaces). More detailshere ;). Edit: typo |
If you have changes to multiple files, editing on GitHub is not a good choice. It creates way too many single commits of one or two minor changes. Please rebase and squash this on your local copy if you can. |
NelleV commentedMar 8, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Can we activate the squash and merge option? This would allow us to squash and merge before merging instead of asking contributors to rebase themselves. ping@tacaswell |
QuLogic commentedMar 8, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
TBH, as acontributor, I don't really like squash&merge as it decouples your fork and upstream's (I don't like rebase&merge for the same reason.) |
Hi, I squashed all 20 commits. Did I do it correctly? Is it reflected in this PR, or should I make a new one? |
Yep, it is done and it works! Thanks! |
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 👍
QuLogic commentedMar 8, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
No, it's not quite right. After rebasing, you need to force push the result. If you don't force, git will complain and suggest you pull everything, but that will restore all the previous commits locally and add another merge commit. So, please do the rebase, and then do |
@yinleon Are you still around Berkeley? I was mistaken and something didn't quite happen right with the rebase. I am happy to help you out with this tomorrow (I should be at BIDS all afternoon if you are around). |
| ax.axis[direction].set_axisline_style("-|>") | ||
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.
There is some whitespace here causing the pep8 failure.
Hi@yinleon This patch needs an additional review. |
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, but can you amend this last typo,@NelleV?
| matplotlib event handling to interact with objects on the canvas | ||
| =========== | ||
| Poly Editor | ||
| ========== |
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.
One missing=.
I think this is ready to be merged.@anntzer can you have a look at this? |
| ==== | ||
| A Matplotlib based game of Pong illustrating one way to write interactive | ||
| animations which are easily ported to multiple backends pipong.py was written |
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.
dot after "multiple backends"
| A Matplotlib based game of Pong illustrating one way to write interactive | ||
| animations which are easily ported to multiple backends pipong.py was written | ||
| by <a href='http://pirsquared.org'>Paul Ivanov</a>. |
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.
hyperlinks should use rst syntax (http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#embedded-uris-and-aliases)
| This is an example of plotting Edward Lorenz's 1963 "Deterministic | ||
| Nonperiodic Flow" in a 3-dimensional space using mplot3d. | ||
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 suggest using an explicit hyperlink target (http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#hyperlink-targets).
| ================= | ||
| This is an example of creating a stacked bar plot with error bars using | ||
| `plt.bar`. Note the parameters `yerr` used for error bars, and `bottom` |
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.
yerr and bottom should use*foo*, not backticks (they are not references to linkable elements).
| ======== | ||
| This example shows how to draw, and manipulate shapes given vertices and nodes | ||
| using the `patches`, `path` and `transforms` classes. |
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.
Patch,Path, andTransform (references to actual python names).
| This shows 4 possible projections using subplot. | ||
| Matplotlib also supports | ||
| <a href='http://matplotlib.org/basemap/'>Basemaps Toolkit</a> and |
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 comment as above regarding hyperlink syntax (either way works).
This is going to take a bit of a rebase now sphinx-gallery is merged. I've assigned this to myself and will try to do it in the next day or so. |
hey@yinleon , a lot has changed with MPL docs in the last few weeks :) This means it'll be a bit more complicated to get this merged. Do you have time to do a live skype where you and I can spot-check these issues and get it back to a mergeable state? |
So I actually had a go at rebasing this last night, and gave up after a while. Quite a lot of the changes have already been made (titles), and there's only a couple of things that are now new unfortunately. I think the easiest thing to do here would be to manually compare the PR diff and the current files, and just copy across anything new. |
Closing in favor of#8860 |
Regarding#7206