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

Path Length Map example #1631

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@kesshijordan
Copy link
Contributor

kesshijordan commented Sep 4, 2018

Added documentation for #1114. I plan to put a preprint up to reference soon (hopefully this week), so I currently have a placeholder in the documentation.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Sep 4, 2018

Codecov Report

Merging #1631 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1631   +/-   ##
=======================================
  Coverage   87.34%   87.34%           
=======================================
  Files         246      246           
  Lines       32265    32265           
  Branches     3504     3504           
=======================================
  Hits        28182    28182           
  Misses       3247     3247           
  Partials      836      836

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 98495ba...b755cd6. Read the comment docs.

@skoudoro
Copy link
Member

skoudoro left a comment

Nice Tutorial, Thank you @kesshijordan. I just had a couple of questions. Otherwise, Can you rebase this PR.

Show resolved Hide resolved doc/examples/path_length_map.py Outdated
interactive window.
"""

interactive = False # this works if it's True but black if False??

This comment has been minimized.

@skoudoro

skoudoro Sep 8, 2018

Member

but black if False?? What do you mean here?
I believe the window just do not appear

This comment has been minimized.

@kesshijordan

kesshijordan Sep 10, 2018

Contributor

The saved png was empty (just black), unless I interacted with it and then closed it, in which case the render was saved. Do you know why this happens? I just tested it again and now it's working, so I'm not sure what's going on. The LiFE tutorial first figure is empty online, so I think this may be a problem in other cases.

This comment has been minimized.

@skoudoro

skoudoro Sep 13, 2018

Member

Strange... I never got an empty png under windows. Thanks for the notice, I will check Life tutorial

nip.plot_stat_map('example_cc_path_length_map.nii.gz',
output_file='Path_Length_Map.png',
bg_img=t1_path, symmetric_cbar=False,
cmap='jet', threshold=1)

This comment has been minimized.

@skoudoro

skoudoro Sep 8, 2018

Member

jet colormap should be avoided apparently. I need to find a good article which explains why.

Can you replace it?

This comment has been minimized.

@kesshijordan

kesshijordan Sep 10, 2018

Contributor

I really like this explanation, too (see below). Absolutely... thanks for the catch : ) Viridis and the other perceptually uniform options are too dark at the bottom of the scale, which is important to stand out in this application, so I changed it to cool but am open to suggestions.

https://twitter.com/choldgraf/status/1006559050169872384?lang=en

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Sep 9, 2018

@kesshijordan kesshijordan force-pushed the kesshijordan:PLM_example branch from 7cd3ef3 to b755cd6 Sep 10, 2018

@skoudoro

This comment has been minimized.

Copy link
Member

skoudoro commented Sep 13, 2018

I need to test it on windows (maybe tomorrow), but it will be good if someone else could look at this PR before merging it

@skoudoro
Copy link
Member

skoudoro left a comment

Finally, Check done under Windows, below my last change requested and then it is ready to go!

seedroi_actor = actor.contour_from_roi(seed_mask, affine,
surface_color, surface_opacity)

ren = window.ren()

This comment has been minimized.

@skoudoro

skoudoro Sep 13, 2018

Member

here, it should be window.Renderer(). Indeed, window.ren() is deprecated

interactive window.
"""

interactive = False # this works if it's True but black if False??

This comment has been minimized.

@skoudoro

skoudoro Sep 13, 2018

Member

Strange... I never got an empty png under windows. Thanks for the notice, I will check Life tutorial

@kesshijordan

This comment has been minimized.

Copy link
Contributor

kesshijordan commented Sep 13, 2018

Thanks for all your work on this, @skoudoro ! I addressed all your comments. I'll add the preprint when it goes up and that's the last bit, I think.

@arokem

arokem approved these changes Sep 13, 2018

Copy link
Member

arokem left a comment

Just a couple of small comments from me. Otherwise - looks good!

"""
==================================
Calculate Path Length Map
(e.g. For Anisotropic Radiation Therapy Contours)

This comment has been minimized.

@arokem

arokem Sep 13, 2018

Member

I would move the parenthetical out of the title and add this as an explanation in the first paragraph below


"""
First, we need to generate some streamlines and visualize. For a more complete
description of these steps, please refer to the CSA Probabilistic Tracking and

This comment has been minimized.

@arokem

arokem Sep 13, 2018

Member

You can cross-link to the other tutorials using the :ref: directive for sphinx.

This comment has been minimized.

@kesshijordan

kesshijordan Sep 14, 2018

Contributor

Thanks, @arokem ! I will do that

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Oct 15, 2018

Superseded by #1649

@arokem arokem closed this Oct 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment