Skip to content

NF: add version of Paul Ivanov's slice viewer #251

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 20 commits into from

Conversation

matthew-brett
Copy link
Member

@ivanov - many thanks for this - it's very nice, and very useful.

The thing I really miss though, is having cross-hairs showing me the
corresponding current point on all three panels.

I think there's an example here:

http://marsbar.sourceforge.net/_images/roi_view.png

I experimented for a while trying to put the crosshairs on, but it was beyond
my blit fu. Any pointers as to how to do that? I mean, put up the
cross-hairs, different on every movement of the slices, but still be able to
blit for speed?

@yarikoptic
Copy link
Member

hm, I thought that such high level functionality as visualization was previously thought worth keeping outside?

@matthew-brett
Copy link
Member Author

Yes, indeed, but then I found I was using it a lot and thought it
would be fairly easy to drop in.

@matthew-brett
Copy link
Member Author

Anyone with matplotlib expertise have a look at this one?

@larsoner
Copy link
Contributor

I might take a look at this to make working on parrec easier, but if
someone else has more mpl experience and/or free time feel free to go for
it instead.

@larsoner
Copy link
Contributor

Done:

matthew-brett#5

While we're at it, it might be worth adding a .plot() method to the image files so that people can easily inspect their data. I don't know the code base well enough to know what layer of abstraction that should live in, though.

@larsoner
Copy link
Contributor

Figured I'd work on this before parrec, since it should facilitate the parrec development :)

@matthew-brett
Copy link
Member Author

Merged your changes, rebasing on master now.

@larsoner
Copy link
Contributor

Great, let me know when you get back to this post-release and I'm happy to review changes you make or go through some iterations with you.

@larsoner
Copy link
Contributor

Should close this given nilearn has something similar, or you think it's still worth pursuing here?

http://nilearn.github.io/building_blocks/plotting.html

@matthew-brett
Copy link
Member Author

Sorry to be lazy - but at a first pass these all looked static - so you can't click around the image?

@larsoner
Copy link
Contributor

It was dynamic last time I used it, clicking around the image should work as should linking two images together, etc. IIRC everything was working correctly when we last looked at it, we just shelved it due to the release and concerns that it should be part of the resampling PR, e.g.:

matthew-brett#5 (comment)

@matthew-brett
Copy link
Member Author

Sorry - I mean the nilearn stuff is static - no? If that is true, this PR still does something we need and that we don't have.

@larsoner
Copy link
Contributor

That's true. Do you think it's worth trying to port our changes over to their code instead of partially duplicating effort? That might be a pain, though, I guess.

How's the resampling coming along? :)

@matthew-brett
Copy link
Member Author

Hi Eric. @choldgraf just caught me in the corridor asking about this one.

I would like to get this merged real-soon-now (TM). I need this stuff too. I think we can sort out the resampling axis stuff later. Do you have some time to rebase this? I've got quite a bit of time in the next few days to do final review.

@larsoner
Copy link
Contributor

larsoner commented Feb 11, 2016 via email

@larsoner
Copy link
Contributor

Closing for #404

@larsoner larsoner closed this Feb 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants