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 save_3d_views and plot_panels functions #337

Merged
merged 3 commits into from Sep 16, 2019

Conversation

@TomDLT
Copy link
Contributor

commented Aug 13, 2019

This PR adds two functions, save_3d_views and plot_panels, to create programmatically figures with multiple views of the same volume. It proposes two layouts as examples.

I added this in a new module, poorly named export. Feel free to suggest a better name.

Layout 1

Figure_1

Layout 2

Figure_2

@sslivkoff

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

looks very nice. and it should be very straightforward to extend when others have custom needs

one nitpick, I am not a huge fan of calling files the same name as their parent dir (cortex/export/export.py) because it makes imports a confusing mess especially if they are preloaded in init.py

return file_names


default_view_params = {

This comment has been minimized.

Copy link
@mvdoc

mvdoc Aug 13, 2019

Contributor

I can see myself wanting to change this (and the following) view parameters. Can these be kwargs with default arguments? (otherwise one would have to modify the python files directly for finer control).

This comment has been minimized.

Copy link
@TomDLT

TomDLT Aug 13, 2019

Author Contributor

Do you have a preference between adding a default_view_params parameter, and doing

cortex.export.export.default_view_params['camera.azimuth'] = 46

?

This comment has been minimized.

Copy link
@mvdoc

mvdoc Aug 13, 2019

Contributor

oh, I didn't know that would work. Then I think it's OK to leave it without extra kwargs.

This comment has been minimized.

Copy link
@sslivkoff

sslivkoff Aug 13, 2019

Contributor

configuration state should be specified in options.cfg

for any dynamic config modification it would be preferable to just pass these things as arguments to the function when the function is called

This comment has been minimized.

Copy link
@mvdoc

mvdoc Aug 13, 2019

Contributor

sorry, actually I should have read the code better. I see that you can pass a dictionary of arguments instead of those default views. So I think you can ignore my comment

This comment has been minimized.

Copy link
@TomDLT

TomDLT Aug 13, 2019

Author Contributor

Yes, I tried to have an easy API for classic views, but something easy to customize if someone wants to go further. I am happy to improve it if you have suggestion !

@mvdoc

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2019

merge?

@mvdoc mvdoc merged commit a672dea into gallantlab:master Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.