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
Added animations for some electromagnetic phenomena #376
Added animations for some electromagnetic phenomena #376
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.
Hey @SunTzunami , thanks for the PR. I liked your examples and would recommend you to rename the folder as refer to skoudoro 's review below. Also prepend your file names, both python scripts and window.record png files with electromagnetic_phenomena
instead of the current caseviz_
in order to help index the generated doc(scripts and png files should have the same name).
Apart from that, according to the right hand thumb rule, I think the direction of your charged particle should be anti-clockwise
instead of it being clockwise
(correct me if I am wrong).
Also I have mentioned some minor changes to fix the formatting below.
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 @SunTzunami,
Really Nice tutorial, Good Job!
Can you remove this subfolder (Electromagnetic Phenomena
). Just let your demo in the examples folder for now. We will reshape the whole documentation later.
Also, it would be good to have those demos with the line actor instead of the point actor.
Can you try it?
# changing with time | ||
|
||
|
||
def update_coordinates(k, w, t, d): |
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 general, you should avoid to have 1 letter as a variable name. Even if it is really nice that you describe them above. Can you update the whole file with the correct name. Thank you!
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 renamed the variables. Thank you!
u = 0.09 | ||
a = 0.004 | ||
t = 0 | ||
dt = 0.09 | ||
w = 0.1 | ||
d = 0.002 |
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 comments as above
|
||
############################################################################### | ||
# end is used to decide when to end the animation | ||
end = 1000 |
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.
For doc generation, can you make it shorter. 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.
I suppose you meant that I reduce the value of end? I reduced end from 1000 to 300.
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.
Yes, and thank you
Hello @SunTzunami! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-02-18 10:49:00 UTC |
Hi @Nibba2018, @skoudoro! I really appreciate the feedback. Thank you! @skoudoro, I renamed the variables and made their names more descriptive. I tried using line actor for the animation but could only implement it for the helical motion animation. |
@SunTzunami Yeah my bad, clockwise direction is correct. |
Thank you for the update. Could you just explain what was your issue with |
Also, Could you create a PR with your gif files in this repo: https://github.com/fury-gl/fury-communication-assets? Thank you @SunTzunami |
Hi @skoudoro ! Thanks for the review! I didn't know how to update coordinates for line actor (in EM wave animation) I then spent quite some time figuring out how to update coordinates for line actor. It works now. :) |
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.
Great, Thank you for the update and for using the line actor.
+1 for merging. Is it good for you too @Nibba2018?
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, there was an error in the EM wave animation (the direction of the arrow was opposite to what it should be). I rectified it.
i guess you haven't pushed these changes. @SunTzunami
@Nibba2018 I did it. PTAL. I changed the camera's location too. |
I made some basic animations using FURY that render some electromagnetic phenomena like-