-
-
Notifications
You must be signed in to change notification settings - Fork 8k
Updated some examples [MEP12] #6486
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.
Can you change this to a for count in range(1000):
That is slightly more pythonic.
c421c66
to
d223623
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.
While you are here, please add relevant spaces around operators.
62c8aac
to
b8ec4d4
Compare
I made the requested changes.
b8ec4d4
to
548d131
Compare
For the PEP8-related failures, I think Travis is simply complaining about this rule "Don't use spaces around the = sign when used to indicate a keyword argument or a default parameter value." (see http://legacy.python.org/dev/peps/pep-0008/#other-recommendations)
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.
@afvincent is correct. Spaces around =
when it is assignment, no spaces when used in a function signature / call for kwargs.
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.
Sorry, forgot to run the pep8 checker before pushing. I've amended the commit, and it looks like it passes now.
e677a95
to
84165bc
Compare
How did you pick up a commit from Christoph?
@adasilva I am not sure what your familiarity with git is. Could you rebase this onto current master and drop that first commit?
Please ask questions if you need help. The rebase guide I like best is still a PR #2742 (I really should finish that).
If this all seems like a disaster and git fights back I can take care of it.
@tacaswell hmm, I'm not sure how that happened. I'm not very confident with rebasing, so let me check in on the process. I started following along the guide you linked to. So far, I added matplotlib/matplotlib as a remote:
git remote add matplotlib https://github.com/matplotlib/matplotlib
Then fetched from the remote:
git fetch matplotlib
Then I started to rebase:
git rebase matplotlib/master
This is the output:
First, rewinding head to replay your work on top of it... Applying: cleaned up animate_decay.py Applying: cleaned up double_pendulum_animated.py Applying: diversified names on vertical axis Applying: changed while loop to for loop in animate_decay.py
Then I looked at the status:
On branch master Your branch and 'origin/master' have diverged, and have 23 and 5 different commits each, respectively. (use "git pull" to merge the remote branch into yours)
Which looks a little different than the output in the doc. Am I still on the right track? In particular, it does not say rebase in process
anywhere in my output.
Actually I might be making this more difficult. I was using rebase to edit the previous commit where there were some small mistakes, and I think that is where I picked up the previous commit. If I run
git rebase -i HEAD~5
then delete the line of the commit that shouldn't be there, will that do what we want?
Yes, i think so.
On Sat, May 28, 2016, 12:37 Ashley DaSilva notifications@github.com wrote:
Actually I might be making this more difficult. I was using rebase to edit
the previous commit where there were some small mistakes, and I think that
is where I picked up the previous commit. If I run
git rebase -i HEAD~5
then delete the line of the commit that shouldn't be there, will that do
what we want?—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#6486 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAMMhWp9mqnZJ4fjXdqsJA3OcsgDlKo5ks5qGG9kgaJpZM4IoFO_
.
@adasilva It sounds like you've got the idea, but you can grab me on facebook chat if it doesn't work. :)
84165bc
to
ebb37af
Compare
ebb37af
to
9619f22
Compare
I removed the extra commit.... any advice on how to resolve the new issues of the failing checks?
You can ignore the test failures. Coveralls have some issues with figuring out what to compare agains (and we don't have tests of examples anyway) and the tests that failed appveyor is known to be flaky.
@jenshnielsen OK thanks!
Replaced by #7329.
Cleaned up examples: animate_decay.py, double_pendulum_animated.py, and barh_demo.py