Add custom_aspect parameter to plot_point_brain. #104
Conversation
Currently, in the plot_point_brain() function, the xyz aspect ratios are manually adjusted to [0.57, 0.57, 0.60]. This manual adjustement provides nice-looking brains that are unfortunately not accurate representations of the actual coordinates of the brain. The way the brain will look will also be totally dependent on the values of the coordinates that we input. This might not be something that we want. I therefore changed the way we adjust the aspect ratio of the different axes such that the ratio is accurately depicting the relationship between the different axes.
|
Hi Vince, I think since last year, Now it seems 3d aspect is fixed in 3.3.0 using It also seems |
|
Ohh Thanks @liuzhenqi77. I did remember that Ross modified the code because at some point, they decided to remove the implementation of
to makes our brains look better: we scale the box aspect (as the name of the function suggests) rather than directly set the limits of the axes. So basically, we get from this (the modified version I proposed above; I added the axes so that we can see what is happening):
to this:
I think that scaling the box aspect is better. And for some reason, the sagittal view now looks much better (I also added a parameter that allows us to choose whether we want our different views to be horizontally aligned or vertically aligned) |
|
To confirm that the set_box_aspect() function works as expected, I looked at what the aspect would look like, for the sagittal view, in 2d space, with aspect='equal':
It looks exactly the same. Since the three views are now much better, I think that we should always want to set the aspect ratio to equal. So I updated my pull request and got rid of the "custom_aspect" parameter and as I mentionned in my previous comment, I added a views_orientation parameter as well. |
Codecov Report
@@ Coverage Diff @@
## master #104 +/- ##
==========================================
- Coverage 72.05% 71.99% -0.07%
==========================================
Files 26 26
Lines 1893 1896 +3
==========================================
+ Hits 1364 1365 +1
- Misses 529 531 +2
Continue to review full report at Codecov.
|
|
This is awesome! 🎉 🙌 Thanks @VinceBaz for the PR and @liuzhenqi77 for digging through the myriad matplotlib issues + errors + updates. I'm happy to take a look at the code but since @liuzhenqi77 was here first maybe you want to take lead on the review? 😁 I've gone ahead and assigned it to you but let me know if you'd prefer not to. |
| if views_orientation == 'vertical': | ||
| ncols, nrows = 1, len(views) | ||
| elif views_orientation == 'horizontal': | ||
| ncols, nrows = len(views), 1 |
There was a problem hiding this comment.
Do you think it would make sense to also change (switch x/y) the figsize if not set explicitly by the user?
There was a problem hiding this comment.
Hum, I am wondering if its ok to have the default parameter being changed without the user being explicitly told about the change. I.E. currently, we set the default figsize to (4, 4.8). If we then change it to (4.8, 4), it might be confusing for the user. What I think could be a good idea is to set this default parameter to None, and then automatically adjust the figsize depending on a) the number of views the user wants, and b) the orientation of the views. For example, if the user wants two views, and horizontal, the figsize would be set to (9.6, 4).
There was a problem hiding this comment.
I replaced the figsize parameter by the views_size parameter (so as not to confuse the user). The actual figsize parameter is adjusted according to the number of views and the orientation of the different views.
-Inspired by zhen-Qi's suggestion, I replaced the figsize parameter of plot_point_brain() with the views_size parameter. This new parameter specifies the size of one single view of the brain. The total figure size will be adjusted based on the number of views that the user wants and the orientation of these views. -Added matplotlib >= 3.3.0 in requirements.txt
|
Looks great to me! Pinging @rmarkello |
The function plot_point_brain() sets the limits of the xyz axes with the following lines of code:
ax.set(xlim=0.57 * np.array(ax.get_xlim()), ylim=0.57 * np.array(ax.get_ylim()), zlim=0.60 * np.array(ax.get_zlim()))It also zscores the coordinates beforehand.
This might be problematic since these limits, being hard-coded, can lead to various bugs. For instance, if we use the coordinates of the Lausanne 1000 parcellation, some parcels are cropped out:
To solve this particular bug, I modified those lines to
ax.set(xlim=0.665 * np.array(ax.get_xlim()), ylim=0.665 * np.array(ax.get_ylim()), zlim=0.70 * np.array(ax.get_zlim())).We now get this when we try to plot the Lausanne 1000 parcellation:
The parcels are no longer cropped out.
Also, I personally enjoy visualizing my data such that the axes are automatically scaled to have "equal" aspect ratios, so I added a custom_aspect parameter to the function that can be set to False if the user wants. If this parameter is set to False, then the axes will be automatically scaled, which gives us:
This comes with the caveat that there is more space between the differerent views, and now the 3 brains are not perfectly aligned on top of one another. Also, the sagittal view is now much more elongated, which might be dissapointing if the user wants to plot a "toy" version of the brain that looks nice. So for this reason, this custom_aspect parameter is set to True by default (which gives us what we had before).