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

feat/Make 3d visualization compatibile with jupyter notebooks #399

Open
wants to merge 7 commits into
base: development
Choose a base branch
from

Conversation

lej0hn
Copy link

@lej0hn lej0hn commented Jul 3, 2024

Aims to solve #359

To run in jupyter you need to get the returned canvas object and call it.
e.g.
canvas = plot_interactive_3D(nml2_doc, plot_type='detailed', axes_pos=[0,0,0], upright=True)
canvas

@sanjayankur31
Copy link
Member

Is it worth creating a plot_interactive_3D_jupyter function that wraps around the plot_interactive_3D function and calls canvas do you think? It'll pass the args to plot_interactive_3D but set the nogui etc. options as required.

Is the rbf package required for this at all? If yes, we'll also need to define a new jupyter extra in setup.cfg that could pull in the necessary deps?

@lej0hn
Copy link
Author

lej0hn commented Jul 3, 2024

While it is possible, then shouldn't we have wrapper functions for each visualization function (plot_3d_cell_morphology, plot_3d_schematic) as well? Also I believe the arguments are identical, so I guess it comes down to which method would be preferred by a user -- to call a different jupyter-specific function or to call canvas? I'm really not sure, although I think I tend a bit to calling canvas.

Yes, the rfb package is required and I believe is the only extra package required. I will add it to the setup.cfg

@sanjayankur31
Copy link
Member

We could have wrappers for all of them yes. I was simply thinking of this:

plot_interactive_3d_jupyter(*args, **kwargs):
    canvas = plot_interactive_3d(*args, **kwargs)
    canvas

(do check the usage of args and kwargs, I can't remember what the correct form is)

because I can't think of a way of informing users that canvas has to be called in jupyter notebooks---it'll have to be documented somewhere and users will have to find that documentation. With the wrapper, all they do is call a function and we take care of things for them "under the hood".

I guess another way is to simply add a new argument to all the functions: jupyter which will call canvas, similar to nogui and all that?

Is calling canvas all that is required? No other changes at all?

@lej0hn
Copy link
Author

lej0hn commented Jul 3, 2024

Yes I see your point. So both the wrappers and the argument are valid solutions, and perhaps with the argument we won't have to return the canvas as well, so it remains more the same.

Apparently yes, calling canvas, or using display(canvas) if in a function are all it needs

@sanjayankur31
Copy link
Member

Tasks:

  • check that all features work in notebooks
  • what happens if the methods are called repeatedly? (does the previous canvas get overwritten??)
  • check if a windowed visualiser can be opened from notebooks: if not, no need to add an argument, just always call canvas if using a notebook; if yes, add an option to allow users to use the windowed visualiser

Use neuroconstruct and play with its visualiser to get an idea of what features we can add to the pyneuroml visualiser

@lej0hn lej0hn force-pushed the feature/jupyter_compatibility branch from ba5a385 to 9560f70 Compare July 10, 2024 15:41
@sanjayankur31 sanjayankur31 marked this pull request as ready for review July 11, 2024 10:27
@sanjayankur31 sanjayankur31 self-requested a review July 11, 2024 10:29
@sanjayankur31 sanjayankur31 self-assigned this Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

None yet

2 participants