-
Notifications
You must be signed in to change notification settings - Fork 270
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
Add ability to spawn code notebooks from cBioPortal queries #4856
Add ability to spawn code notebooks from cBioPortal queries #4856
Conversation
✅ Deploy Preview for cbioportalfrontend ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Nice and quick work @gautamsarawagi ! Couple things:
|
@alisman You are absolutely right, We will need to host the JupyterLite ourself to pass the data to it.
Have got the direction now. Will keep you updated on progress. |
@alisman I have read all the docs in this last week. and tried out different cases:
I would love to hear your thoughts on this and how I should move further. |
@gautamsarawagi. Looking good! I like seeing the real data in the interface. Perhaps we can schedule a zoom session to meet eachother and try to talk about next steps. Thank you for your work! Are you hoping to do GSOC this year? --Aaron |
@alisman, thanks for the feedback! The extension does seem like the way to go for smooth communication between the portal and the notebook. A Zoom session is a great idea to meet each other and discuss the next steps. Please do let me know your availability and I'll make sure to accommodate accordingly. And yes, I'm hoping to do GSOC this year! This project is right up my alley, so I'd love to contribute more if selected. Appreciate you taking the time to review and provide helpful comments. Looking forward to our discussion and potential collaboration over the summer! Regards, |
Hey @alisman based on our discussion yesterday I came across this extension from the jupyterlab itself. And you were right. They are using ServiceManager API's - index.ts This might be a good resource. Also, I came across this discussion this gives a good understanding of how the files are stored internally. I am working on this and will update you once the integration is successful. |
@gautamsarawagi do you want me to look at anything in particular yet? |
Based on our discussion I came out with some similar available resources. So, actually wanted to share those. |
@gautamsarawagi i think it would be good to include the notebook code itself in the PR. We can figure out a way get the js loaded into an HTML page on the cbioportal.org domain. THat will make progress and review quicker. Ultimately, we may choose for security reasons to host the notebook on a different domain. But the js itself can still live in this repository for convenience. |
@alisman thats actually a good idea. It would be good to add the notebook code in the PR itself. |
@alisman I have added the notebook in the root directory of the code itself. and have also specified a Readme.md to use it. Although I was not able to reference the index.html from the root directory to the JupyterNotebookTool.tsx : I would also like to discuss this with you.. I even tried adding html-loaders in the So finally I deployed it for the use If we can reference the notebook's index.html in the iframe then there won't be a need for the deployment which will help us tackle the security factors that are coming right now. Also, I am now moving to my further work of working with the fileModification. Please do let me know if any further modifications are needed. |
@gautamsarawagi i tested this and it looks good. Here are some next steps:
|
Sure @alisman. First of all thanks for the feedback from your end. I will start working on showing a simple python file containing the file and running it. Will share the progress with you. |
Hey @alisman
Will update you once the auto-execution part is done. PFA: The changes in the extension created for the file-communication: link |
fieldsToKeep | ||
); | ||
|
||
this.jupyterFileContent = allGenesMutationsCsv; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should delete jupyterFileContent
this when the modal is closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alisman I have updated the code.
Jupyter-Notebook-lite in the frotnend for this issue
Describe changes proposed in this pull request:
Checks
Any screenshots or GIFs?
If this is a new visual feature please add a before/after screenshot or gif
here with e.g. Giphy CAPTURE or Peek
Notify reviewers
Read our Pull request merging
policy. It can help to figure out who worked on the
file before you. Please use
git blame <filename>
to determine thatand notify them either through slack or by assigning them as a reviewer on the PR