-
Notifications
You must be signed in to change notification settings - Fork 114
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
Visualize pipeline objects in notebook #2241
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
…at/umd-viz-bundle Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
…at/umd-viz-bundle Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
…at/viz-pipe Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: ravi_kumar_pilla <[email protected]>
@astrojuanlu for security reason (accessing localStorage), esm bundle does not allow globalNavigation to be On databricks: ![]() |
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Hi @astrojuanlu , Wow ! Thanks for the quick test and feedback. I can check if we can -
I will fix 1, 2 for now |
Signed-off-by: ravi_kumar_pilla <[email protected]>
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.
Hi @ravi-kumar-pilla ,
Does it make sense to separate how we:
- Load Kedro-Viz from a Kedro project via a FastAPI server
- Load Kedro-Viz in a notebook by generating JSON and bundling it using ESM
Currently, notebook-related functions are in data_loader.py and server.py, making these files larger and somewhat out of place. Would it be better to create a new folder under integrations called notebooks and move the visualizer and loader files there for better separation?
Let me know your thoughts!
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
html_content = self.generate_html( | ||
json_to_visualize, self.options, self.js_url | ||
) | ||
iframe_content = self._wrap_in_iframe( |
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.
do we need to do this after, can we not add heigh/width in generate_html itself
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.
I am not sure about the question but the height and width are used to customize the iframe size. Do you think we should customize the root div
instead ? or both iframe and root div
?
Signed-off-by: ravi_kumar_pilla <[email protected]>
1 more thing @ravi-kumar-pilla : do you think having
would make
work with what's currently in this PR? (That way the user doesn't even have to do any extra imports for this to work) |
Love the idea. Sure we can do that ! |
Signed-off-by: ravi_kumar_pilla <[email protected]>
Signed-off-by: ravi_kumar_pilla <[email protected]>
Description
Resolves #1993
NOTE: The bundle URL will be updated once #2268 is merged
Development notes
NotebookVisualizer
and a methodshow
responsible for visualizing Kedro-Viz using the esm bundle in notebookload_data_for_notebook_users
andload_and_populate_data_for_notebook_users
methods to kedro-viz -> integrations -> notebook ->data_loader.py
QA notes
Testing Results :
Jupyter lab:
Databricks:
Marimo:
![image](https://private-user-images.githubusercontent.com/87735534/412167045-87bab2fb-391c-4b0e-b669-35d4f7683a7c.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1NzA5NjksIm5iZiI6MTczOTU3MDY2OSwicGF0aCI6Ii84NzczNTUzNC80MTIxNjcwNDUtODdiYWIyZmItMzkxYy00YjBlLWI2NjktMzVkNGY3NjgzYTdjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTQlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE0VDIyMDQyOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWFkYzg3OGMxMjI2MjAwY2JiNTAxNzE2NDgwNzJkN2U2YTQwZjRmMjVlMDNjOGY1OWFkMmRlMzBlNGQxZDVkZTQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.ey-XTYgsSpuhKQHP7B3VaK-MqGeDJLytJJCaNjrLXxw)
VS Code
![image](https://private-user-images.githubusercontent.com/87735534/412168799-81bde634-5ebe-43f8-87b5-80ed1aaa7853.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1NzA5NjksIm5iZiI6MTczOTU3MDY2OSwicGF0aCI6Ii84NzczNTUzNC80MTIxNjg3OTktODFiZGU2MzQtNWViZS00M2Y4LTg3YjUtODBlZDFhYWE3ODUzLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTQlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE0VDIyMDQyOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWVmNGE5ODI1ZTUwZjBjZTYyMmM0YTM0M2NkYjA5ZGNiNGE2Mjk2ODQwYzEyOGM3ZDY4NjIxZjY5YWRiNDUwNjMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.kdJ3caDDec8kYSjaqCySEf1E02MzgJjAGgocAgRkxGk)
Checklist
RELEASE.md
file