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

A lightweight UI for medical visualizations #4: 2D Line Slider #1205

Merged
merged 15 commits into from Apr 9, 2017

Conversation

Projects
None yet
5 participants
@ranveeraggarwal
Member

ranveeraggarwal commented Mar 30, 2017

Continuing from #1199 and the fourth installment of #1111, this PR adds a Line Slider element to the Viz module.

Complete blogpost here: http://ranveeraggarwal.com/blog/gsoc-2016-summary

@codecov-io

This comment has been minimized.

codecov-io commented Mar 30, 2017

Codecov Report

Merging #1205 into master will increase coverage by 0.03%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1205      +/-   ##
==========================================
+ Coverage   85.87%   85.91%   +0.03%     
==========================================
  Files         221      221              
  Lines       27116    27247     +131     
  Branches     2776     2782       +6     
==========================================
+ Hits        23285    23408     +123     
- Misses       3148     3154       +6     
- Partials      683      685       +2
Impacted Files Coverage Δ
dipy/viz/tests/test_ui.py 85.79% <72.72%> (-1.79%) ⬇️
dipy/viz/ui.py 90.77% <97.95%> (+1.56%) ⬆️
dipy/reconst/dki.py 97.63% <0%> (ø) ⬆️
dipy/denoise/non_local_means.py 100% <0%> (ø) ⬆️
dipy/reconst/cache.py 100% <0%> (ø) ⬆️
dipy/reconst/base.py 88.88% <0%> (ø) ⬆️
dipy/reconst/csdeconv.py 88.52% <0%> (ø) ⬆️
dipy/tracking/tests/test_streamline.py 98.14% <0%> (ø) ⬆️
dipy/segment/tests/test_quickbundles.py 97.95% <0%> (+0.02%) ⬆️
dipy/align/reslice.py 97.29% <0%> (+0.23%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b201a0a...60e469b. Read the comment docs.

@coveralls

This comment has been minimized.

coveralls commented Mar 30, 2017

Coverage Status

Coverage increased (+0.03%) to 88.407% when pulling e0d708c on ranveeraggarwal:ui into b201a0a on nipy:master.

@MarcCote

I think it would be more user-friendly we could manually set not only the percentage but also the directly a value (assuming we defined some min/max values for the slider in the first hand). Also, it would be nice to have an easy of modifying the text that is displayed next to the slider.

Actually, I've already done those improvements but for an older version of the UI API. You can clearly reuse a lot of stuff from here
https://github.com/MarcCote/dipy/blob/interactive_clustering/dipy/viz/gui_2d.py#L843

Look at the following new methods: update format_text, set_ratio, set_value. Also, I've modified __init__ and set_position (and maybe set_center).

I think it would be awesome to have those features. I might have time this weekend to make a PR on top of yours if you prefer. Let me know.

Other than that, there are small PEP8 issues.

# Slider Line
self.slider_line = Rectangle2D(size=(self.length, self.line_width), center=self.center).actor
self.slider_line.GetProperty().SetColor(1, 0, 0)

This comment has been minimized.

@MarcCote

MarcCote Mar 31, 2017

Contributor

Remove empty line.

self.slider_disk.SetMapper(mapper)

self.slider_disk.SetPosition(self.center[0], self.center[1])

This comment has been minimized.

@MarcCote

MarcCote Mar 31, 2017

Contributor

Empty line

percentage = 0
if percentage > 100:
percentage = 100
return str(percentage) + "%"

This comment has been minimized.

@MarcCote

MarcCote Mar 31, 2017

Contributor

Add a new line before return.

The new center of the whole slider (x, y).
"""
self.slider_line.SetPosition(position[0] - self.length / 2, position[1] - self.line_width / 2)

This comment has been minimized.

@MarcCote

MarcCote Mar 31, 2017

Contributor

Max character per line should be 80 chars. Run pep8 ui.py and flakes8 ui.py, nothing should output to the console. if the code respects the PEP8 standard.

@ranveeraggarwal

This comment has been minimized.

Member

ranveeraggarwal commented Mar 31, 2017

@MarcCote great idea. Please do make a PR. In case you're occupied, I can make add those changes too.

@coveralls

This comment has been minimized.

coveralls commented Mar 31, 2017

Coverage Status

Coverage increased (+0.03%) to 88.407% when pulling 371b6d9 on ranveeraggarwal:ui into b201a0a on nipy:master.

MarcCote and others added some commits Apr 1, 2017

Merge pull request #11 from MarcCote/ui_enh_slider
ENH: Can now use value or ratio to set automatically the position of …
@coveralls

This comment has been minimized.

coveralls commented Apr 1, 2017

Coverage Status

Coverage increased (+0.03%) to 88.41% when pulling eac3e05 on ranveeraggarwal:ui into b201a0a on nipy:master.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Apr 7, 2017

Hi @ranveeraggarwal I am getting this error on windows 10

C:\Users\elef\Devel\dipy\doc\examples>python viz_ui.py
Traceback (most recent call last):
  File "viz_ui.py", line 102, in <module>
    line_slider = ui.LineSlider2D()
  File "c:\users\elef\devel\dipy\dipy\viz\ui.py", line 1379, in __init__
    outer_radius=outer_radius, text_size=text_size)
  File "c:\users\elef\devel\dipy\dipy\viz\ui.py", line 1426, in build_actors
    self.text.position = (self.left_x_position - 50, self.center[1] - 10)
  File "c:\users\elef\devel\dipy\dipy\viz\ui.py", line 922, in position
    self.actor.SetDisplayPosition(*position)
TypeError: SetDisplayPosition argument 1: integer argument expected, got float

My vtk version is 6.3.0 in this machine.

@ranveeraggarwal

This comment has been minimized.

Member

ranveeraggarwal commented Apr 7, 2017

@Garyfallidis this was a bug in the TextActor2D that carried over. Instead of setting the position, I was setting the display coordinates previously, which are (int, int).

I think it'll work now, please verify.

@coveralls

This comment has been minimized.

coveralls commented Apr 7, 2017

Coverage Status

Coverage increased (+0.03%) to 88.412% when pulling 10dd142 on ranveeraggarwal:ui into b201a0a on nipy:master.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Apr 7, 2017

Hi @ranveeraggarwal you need to fetch the viz icons first and then read them.

@ranveeraggarwal

This comment has been minimized.

Member

ranveeraggarwal commented Apr 7, 2017

@Garyfallidis I guess that got missed somewhere during refactoring. Fixed now.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Apr 7, 2017

Please also remove all unnecessary comments.
# /Panel

@ranveeraggarwal

This comment has been minimized.

Member

ranveeraggarwal commented Apr 7, 2017

@Garyfallidis done, also added some docstrings for documentation.

@coveralls

This comment has been minimized.

coveralls commented Apr 7, 2017

Coverage Status

Coverage increased (+0.03%) to 88.412% when pulling f77d591 on ranveeraggarwal:ui into b201a0a on nipy:master.

@coveralls

This comment has been minimized.

coveralls commented Apr 7, 2017

Coverage Status

Coverage increased (+0.03%) to 88.412% when pulling f77d591 on ranveeraggarwal:ui into b201a0a on nipy:master.

This example shows how to use the UI API.
Currently includes button, textbox, panel, and line slider.
First, a bunch of imports.

This comment has been minimized.

@Garyfallidis

Garyfallidis Apr 9, 2017

Member

Remove this comment and check again which of the imports you really need for this tutorial.
Do you really need all these imports? Checking if vtk is available?

This comment has been minimized.

@ranveeraggarwal

ranveeraggarwal Apr 9, 2017

Member

We do need vtk to make those cubes. Or should I remove the cubes?

This comment has been minimized.

@Garyfallidis

Garyfallidis Apr 9, 2017

Member

You can keep the cubes for now.

@@ -19,8 +29,14 @@

numpy_support, have_ns, _ = optional_package('vtk.util.numpy_support')

This comment has been minimized.

@Garyfallidis

Garyfallidis Apr 9, 2017

Member

Do you really need this in the tutorial?

current_size = (600, 600)
show_manager = window.ShowManager(size=current_size, title="DIPY UI Example")

show_manager.ren.add(cube_actor_1)
show_manager.ren.add(cube_actor_2)
show_manager.ren.add(panel)
show_manager.ren.add(text)
show_manager.ren.add(line_slider)

show_manager.start()

This comment has been minimized.

@Garyfallidis

Garyfallidis Apr 9, 2017

Member

We need to comment this. Otherwise the tutorial will block execution of other tutorials when the documentation is generated.
Also, we need to update viz_advanced.py and viz_widgets.py to use the new line_slider. Basically we need to eradicate widgets from the tutorials.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Apr 9, 2017

The tutorials needs to be connected to examples_index.rst and valid_examples.txt

"""

from dipy.data import read_viz_icons, fetch_viz_icons

# Conditional import machinery for vtk.
from dipy.utils.optpkg import optional_package

This comment has been minimized.

@Garyfallidis

Garyfallidis Apr 9, 2017

Member

But do remove this import about optional package.
You can use

from dipy.viz.window import vtk

This comment has been minimized.

@ranveeraggarwal

ranveeraggarwal Apr 9, 2017

Member

Sure, I'll add this.

@@ -10,17 +20,14 @@

vtk, have_vtk, setup_module = optional_package('vtk')

This comment has been minimized.

@Garyfallidis

Garyfallidis Apr 9, 2017

Member

Remove this too please.

@coveralls

This comment has been minimized.

coveralls commented Apr 9, 2017

Coverage Status

Coverage increased (+0.03%) to 88.415% when pulling 8c8eda8 on ranveeraggarwal:ui into b201a0a on nipy:master.

@coveralls

This comment has been minimized.

coveralls commented Apr 9, 2017

Coverage Status

Coverage increased (+0.03%) to 88.415% when pulling 60e469b on ranveeraggarwal:ui into b201a0a on nipy:master.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Apr 9, 2017

Great! Thanks @ranveeraggarwal. I will cleanup the widgets and while you are working on the circular slider and filedialog? I suggest to add the circular slider in this tutorial. No need for a new example. Perhaps the filedialog too.

@Garyfallidis Garyfallidis merged commit f6c8dee into nipy:master Apr 9, 2017

4 checks passed

codecov/patch 93.33% of diff hit (target 85.87%)
Details
codecov/project 85.91% (+0.03%) compared to b201a0a
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.03%) to 88.415%
Details
@ranveeraggarwal

This comment has been minimized.

Member

ranveeraggarwal commented Apr 10, 2017

@Garyfallidis yep sure 😄

ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018

Merge pull request nipy#1205 from ranveeraggarwal/ui
A lightweight UI for medical visualizations nipy#4: 2D Line Slider
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment