Skip to content
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

ENH: Add different orientations to CSD Episode #69

Merged
merged 10 commits into from
Mar 19, 2021

Conversation

kaitj
Copy link
Collaborator

@kaitj kaitj commented Mar 16, 2021

Resolves #29. Adds in the different orientations for the fodfs and peaks figures. Made use of matplotlib subplots to save figures rather than the fury scene. One change was to update the processing from only a single 30x30x1 slice to 30x30x30 volume. Takes a little bit more time, but still quick enough for a lesson I believe. Also updated the processing to use the rotated bvecs following eddy.

@jhlegarreta May want to double check the orientations - I did get a little turned around with the rotations. Not the most familiar with fury, but hopefully I got it right!

@netlify
Copy link

netlify bot commented Mar 16, 2021

Deploy preview for carpentries-dmri ready!

Built with commit d63a504

https://deploy-preview-69--carpentries-dmri.netlify.app

Copy link
Contributor

@jhlegarreta jhlegarreta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaitj thanks for giving this a try.

I usually move the camera instead of the data to show different orientations, but my approach is a little bit more convoluted. As the figures look now, I am unable to say whether the rotations are correct. Maybe we need to do the computations on a volume that can be more easily identified in all axes? If you are sure that the current orientations are correct, we can merge this in order to push things forward and re-define the region in a separate PR.

Also, in a separate PR, we may want to:

  • Put the parts that produce the plots in different orientations to a method so that we avoid repeating the code, and maybe be able to use import it for the tractography notebooks.
  • Maybe set the slice we want to show as the half slice in every axis (i.e. computing it from the dimensions).

@kaitj
Copy link
Collaborator Author

kaitj commented Mar 17, 2021

I can definitely give it a try with the camera and where how that goes. As it currently stands, it is mostly capturing cortex without any identifiable structure. One thought is to maybe also include an anatomical image underneath? I am pretty confident the orientations are correct, but would feel better about either identifiable features or an underlying image.

Regarding a separate method, I like the idea and should be further discussed.

Regarding the half slice, I had thought about that as well. In this case, the half slice in the 30x30x30 volume, but it went back to not really able to tell what what exactly we were looking at. Again, this can likely be fixed by switching to a different volume or an underlying anatomical image.

All that said, I will play around with moving the camera!

@kaitj
Copy link
Collaborator Author

kaitj commented Mar 17, 2021

I've updated (locally so far) to focus on the volume to the splenium. Should be easier to determine orientations. This also meant changing the volume from 30x30x30 to 40x40x10.

@kaitj
Copy link
Collaborator Author

kaitj commented Mar 17, 2021

d63a504 updates the code to grab orientation screenshots surrounding the splenium. It makes it much easier to identify the orientations than before based off of what know of how the odfs should be. Screenshots are grabbed from the mid slice of each small volume. It also moves away from rotating the data to rotating the camera (we should probably include something about the roll, pitch, and yaw).

@kaitj kaitj requested a review from jhlegarreta March 17, 2021 16:18
Copy link
Contributor

@jhlegarreta jhlegarreta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaitj looks 💯. The selected is area can now be clearly identified. Thanks for all the effort.

I'd leave for a separate PR putting the parts dealing with the viewpoint to a function that an be re-used.

(we should probably include something about the roll, pitch, and yaw).

Not sure what you mean by this.

@kaitj
Copy link
Collaborator Author

kaitj commented Mar 18, 2021

I don't recall now if one of the previous notebooks had anything about rotations for transforms, but I was thinking about something like the following image:

image

Just to give a clearer picture of what might be happening in the code when scene.yaw or scene.roll is called.

@jhlegarreta
Copy link
Contributor

@kaitj Definitely looks helpful. In my mind, these might fit better in some of the contents in Extras (e.g. Discussion or a new one that we may propose), since generating the appropriate viewpoints are not the focus of this or other lessons. Also, if all camera set up calls get into a function, they are less likely to represent an obstacle to go through the episode and clearly understand it thoroughly a hindrance.

If the image posted has the appropriate permissions that allows to be re-used, I would vote for merging the PR and leave putting all calls into a function and adding the necessary explanations in a section in Extras in a separate PR.

@kaitj
Copy link
Collaborator Author

kaitj commented Mar 18, 2021

Definitely looks helpful. In my mind, these might fit better in some of the contents in Extras (e.g. Discussion or a new one that we may propose), since generating the appropriate viewpoints are not the focus of this or other lessons. Also, if all camera set up calls get into a function, they are less likely to represent an obstacle to go through the episode and clearly understand it thoroughly a hindrance.

Agreed that it this is not the focus and probably fits better under extras.

If the image posted has the appropriate permissions that allows to be re-used, I would vote for merging the PR and leave putting all calls into a function and adding the necessary explanations in a section in Extras in a separate PR.

We would have to get permission or create a new image. This was just used here for a quick example. I think we are good for merging either way as it is not in the episode itself and something to be discussed about where it best fits.

@kaitj kaitj added the type:enhancement Propose enhancement to the lesson label Mar 18, 2021
@jhlegarreta
Copy link
Contributor

OK, so I'll go ahead and merge. We will find later. the appropriate place for this or another figure that we can use together with the accompanying explanation.

Thanks for having worked on this 💯.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement Propose enhancement to the lesson
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve how the CSD reconstruction lesson screenshots are displayed
2 participants