Skip to content

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

Closed
wants to merge 4 commits into from
Closed

Conversation

adasilva
Copy link

Cleaned up examples: animate_decay.py, double_pendulum_animated.py, and barh_demo.py

Generates a decaying sine wave.
"""
count = 0
while count < 1000:
Copy link
Member

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.

M2*G*sin(state[2])*cos(del_) +
M2*L2*state[3]*state[3]*sin(del_) -
(M1 + M2)*G*sin(state[0]))/den1
den1 = (mass1 + mass2)*length1 - mass2*length1*np.cos(del_)*np.cos(del_)
Copy link
Member

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.

@tacaswell tacaswell added this to the 2.1 (next point release) milestone May 27, 2016
@adasilva
Copy link
Author

I made the requested changes.

@afvincent
Copy link
Contributor

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)

return line, time_text

ani = animation.FuncAnimation(fig, animate, np.arange(1, len(y)),
interval=25, blit=True, init_func=init)
interval = 25, blit = True, init_func = init)
Copy link
Member

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.

Copy link
Author

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.

@adasilva adasilva force-pushed the master branch 2 times, most recently from e677a95 to 84165bc Compare May 28, 2016 13:47
@tacaswell
Copy link
Member

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.

@adasilva
Copy link
Author

@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.

@adasilva
Copy link
Author

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?

@tacaswell
Copy link
Member

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_
.

@TrishGillett
Copy link
Contributor

@adasilva It sounds like you've got the idea, but you can grab me on facebook chat if it doesn't work. :)

@adasilva
Copy link
Author

adasilva commented Jun 2, 2016

I removed the extra commit.... any advice on how to resolve the new issues of the failing checks?

@jenshnielsen
Copy link
Member

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.

@adasilva
Copy link
Author

adasilva commented Jun 2, 2016

@jenshnielsen OK thanks!

@NelleV NelleV added the MEP: MEP12 gallery and examples improvements label Oct 3, 2016
@QuLogic
Copy link
Member

QuLogic commented Nov 24, 2016

Replaced by #7329.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MEP: MEP12 gallery and examples improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants