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 tutorial for RadioButton and CheckBox UI #208
Conversation
Hello @PaulNicolasHunter! 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 2020-03-20 10:32:25 UTC |
Codecov Report
@@ Coverage Diff @@
## master #208 +/- ##
=======================================
Coverage 88.63% 88.63%
=======================================
Files 17 17
Lines 4347 4347
Branches 547 547
=======================================
Hits 3853 3853
Misses 354 354
Partials 140 140 |
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 is a really nice tutorial, I like it a lot. Thank you!
Concerning the part 2 of this tutorial. I proposed you to add CheckBox
. Each checkbox will be a shape (cube, sphere, cone, arrow) that you can control to make these actors visible or not. You can keep the radiobutton to change the color of all elements
|
||
######################################################################## | ||
# Cube and Radio Buttons | ||
# ================ |
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.
add ===
until the end of Buttons
|
||
############################################################################### | ||
# Show Manager | ||
# ============================================================================ |
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.
Remove some =====
. It should be the size of ShowMAnager
# manager. | ||
|
||
current_size = (800, 800) | ||
show_manager = window.ShowManager(size=current_size, title="DIPY Cube Example") |
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.
FURY
instead of DIPY
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.
Got it, I'll push the suggested changes along with part 2 of this tutorial.
@skoudoro, I've created the tutorial as you suggested, kindly, let me know if you want me to change anything or improvise it further. |
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.
Thank you for the update. See below my comments.
height=20) | ||
arrow = arrow_maker(color=(0, 0, 1), start_point=(0, 25, 0), | ||
end_point=(40, 25, 0), shaft_resolution=50, | ||
tip_resolution=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.
Can you remove cube_maker
, sphere_make
r, cone_maker
, arrow_make
r and replace them by
actor.cone
, actor.cube
, actor.sphere
, actor.arrow
.
you need to import actor
module on the top
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.
That's cool, I'll have a look at it.
But then why are we not using actor.cube
in other tutorials as well?
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 because they are old and they need to be updated 😄
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.
cool, I was looking at the actor
earlier while coding the tutorials and was wondering why are we not incorporating this instead.
In this case, I should update viz_ui.py
and viz_ui_slider.py
as well, maybe in a separate PR.
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.
@skoudoro I'm unable to toggle colors of the figures when using actor.cube
.
I've checked the value of colors is being set using cube.GetProperty().GetColor()
but it's not reflecting on the window.
what could be the reason? maybe the actors which are returned are the reason?
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.
Indeed, it is much more complicated to update the colors. We need to create some functions/interfaces to make it easier. So, keep it as it is for the moment and just do the other corrections.
To get the color, you can use cube.GetMapper().GetColorMapColors()
but it is not convenient to update it. It needs some thinking.
Thank you @PaulNicolasHunter !
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.
Alright, I understand @skoudoro, I'll try to dig more into it and find a workaround as soon as possible.
# ========================= | ||
|
||
|
||
def arrow_maker(color=(1, 1, 1), start_point=(0, 25, 0), |
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.
no need of this function, it is on the fury.actor
module. Please, remove it.
# ======================= | ||
|
||
|
||
def cone_maker(color=(1, 1, 1), radius=5.0, center=(0, 0, 0), |
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.
no need of this function, it is on the fury.actor
module. Please, remove it.
This is the first part of the tutorial in reference to issue number #205.
This tutorial demonstrates the use of RadioButton UI, we can toggle the colors of the cube using the given options.