Brain GUI modernization (Phase 4) - #14102
Conversation
| assert brain._alpha == 0.5 | ||
|
|
||
| brain.set_cortex_colormap("bone") | ||
| assert brain._cortex_preset == "bone" |
There was a problem hiding this comment.
this doesn't test whether the new colors are actually shown, like you do below with changing surf
| rng=rng, | ||
| opacity=opacity, | ||
| name=name, | ||
| smooth=smooth, |
There was a problem hiding this comment.
Why a new smooth parameter? We already had controls for the amount of smoothing, which you could set to 1 to turn it off.
There was a problem hiding this comment.
smooth_mat lives on the whole mesh but the new smooth just tracks per-overlay.
|
ready to be reviewed @wmvanvliet , tnx in advance, phase 5 is almost ready ... |
|
When selecting some vertices, the spheres don't update when switching surfaces. Have you tried this with a |
|
Sharp eyes! you can now test it with this small script: |
| for sphere in spheres: | ||
| mesh = sphere["mesh"] | ||
| mesh.points = mesh.points + (center - np.array(mesh.center)) | ||
|
|
There was a problem hiding this comment.
Does this need to be an entire separate method if it's only called once?
There was a problem hiding this comment.
Fair point, if it was only one method, I would fold it into set_surf but now we have two (_reposition_vertex_glyphs + _reposition_vector_glyphs), just for better readability ...
There was a problem hiding this comment.
Sorry, but I disagree that splitting off that functionality in two private method, both of which are only called once, is more readable. _brain.py currently sits at 4859 lines with 65 helper functions. It's fair to say this thing has gotten a bit out of control, hasn't it? This is why I'm not a big fan of adding even more helper functions if not really necessary. In this case, the functions only encapsulate a few lines of code that I don't think hurt much if just left in brain.set_surf.
There was a problem hiding this comment.
Agreed. Done.
Maybe we can check and trim some helper functions later (in another PR).
|
Looks like we just need a changelog entry and then it's good to go from my end. |
|
Thanks @wmvanvliet |
|
Thanks, @payamsash! |
Reference issue (if any)
Phase 4 of #14042 and following #14085
What does this implement/fix?
add
surfandcortexoptions to GUIAdditional information
Claude reviewed the code and I understand what has been modified.