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

%load_node does not work with notebook>=7.2.0 #4369

Closed
marrrcin opened this issue Dec 5, 2024 · 12 comments · Fixed by #4540
Closed

%load_node does not work with notebook>=7.2.0 #4369

marrrcin opened this issue Dec 5, 2024 · 12 comments · Fixed by #4540
Assignees
Labels
Issue: Bug Report 🐞 Bug that needs to be fixed

Comments

@marrrcin
Copy link
Contributor

marrrcin commented Dec 5, 2024

Description

Based on the latest guide on main branch I've tried running notebook with Kedro kernel. While most of the functionality seems OK, the %load_node feature does not seem to work correctly.

Context

Not being able to use %load_node in Jupyter Notebook with Kedro loaded.

Steps to Reproduce

  1. Follow instructions from latest guide on main branch
  2. Run kedro jupyter notebook
  3. In new notebook, run %load_node split_data_node

Expected Result

Function works properly as with notebook==7.1.2.
Screenshot 2024-12-05 at 22 28 34

Actual Result

Screenshot 2024-12-05 at 22 30 17

Additional info

Works fine when version of notebook is pinned to 7.1.2. I did not do bisection between 7.1.2 and 7.3.1 to find where it broke though.

Your Environment

  • Kedro version used (pip show kedro or kedro -V): 0.19.10
  • Python version used (python -V): Python 3.10.10
  • Operating system and version: macOS 14.3.1 (23D60) (ARM)
@astrojuanlu
Copy link
Member

Thank you for reporting this @marrrcin ! We'll have somebody else on the team try to reproduce this.

@DimedS
Copy link
Member

DimedS commented Jan 30, 2025

Thanks, @marrrcin. I confirm that starting from Notebook 7.2.0, this feature stopped working. It creates notebook cells, but they are empty instead of containing the node code.

@DimedS DimedS changed the title %load_node does not work with notebook>=7.3.1 %load_node does not work with notebook>=7.2.0 Jan 30, 2025
@DimedS DimedS removed this from Kedro 🔶 Jan 30, 2025
@DimedS DimedS added the Issue: Bug Report 🐞 Bug that needs to be fixed label Jan 30, 2025
@SajidAlamQB SajidAlamQB self-assigned this Feb 28, 2025
@SajidAlamQB SajidAlamQB moved this to In Progress in Kedro 🔶 Feb 28, 2025
@SajidAlamQB
Copy link
Contributor

SajidAlamQB commented Feb 28, 2025

After some digging, it appears that a recent change to Jupyter Notebook (version 7.2 and above) introduced a feature called “Full notebook windowing mode” which is now enabled by default:

Changelog reference

This mode virtualises how cells are rendered, so only visible cells are injected into the DOM. As a result, any extension or magic command that relies on inserting new cells (e.g. via DOM manipulation or ipylab commands) may fail silently or have broken behavior.

In our case, the %load_node command from Kedro depends on ipylab commands, which used to work fine in older Notebook versions but now no longer responds in Notebook 7.2+ when “Full windowing mode” is active.

Workaround / Solution:

We can actually revert to the older behaviour through settings on jupyter notebook, change the “Windowing mode” setting from “full” to “defer.”:

  1. Open your Notebook.
  2. Go to Settings → Settings Editor → Notebook in the menu.
  3. Scroll to “Windowing mode” and choose defer.
Image

This should allows the %load_node command to work again as it did previously.

@DimedS @astrojuanlu @merelcht should we look into how to set this globally or programmatically or maybe just update our docs?

@astrojuanlu
Copy link
Member

Ugh, that's really unfortunate... Thanks for digging @SajidAlamQB , this is helpful!

IMHO, if it doesn't work with the default settings it's unfortunately broken. Can we maybe rework the feature so that instead of inserting new cells, it writes everything to the current one?

@DimedS
Copy link
Member

DimedS commented Feb 28, 2025

Ugh, that's really unfortunate... Thanks for digging @SajidAlamQB , this is helpful!

IMHO, if it doesn't work with the default settings it's unfortunately broken. Can we maybe rework the feature so that instead of inserting new cells, it writes everything to the current one?

I like that proposal. It might also be possible to switch off the Full Notebook Windowing mode programmatically when running the %load_node command or modify the API we use to allow creating a new cell even with that mode enabled.

@SajidAlamQB
Copy link
Contributor

SajidAlamQB commented Feb 28, 2025

It might also be possible to switch off the Full Notebook Windowing mode programmatically when running the %load_node command.

This solution could work, we would need to temporarily change the windowing mode to "defer" before running the cell creation commands, then restore it afterward. It will maintain compatibility with all notebook versions and doesn't require users to manually change settings and keeps the default option albeit a little bit hacky.

@DimedS
Copy link
Member

DimedS commented Feb 28, 2025

This solution could work, we would need to temporarily change the windowing mode to "defer" before running the cell creation commands, then restore it afterward. It will maintain compatibility with all notebook versions and doesn't require users to manually change settings and keeps the default option albeit a little bit hacky.

looks like easiest solution if it will work

@SajidAlamQB
Copy link
Contributor

Can we maybe rework the feature so that instead of inserting new cells, it writes everything to the current one?

@astrojuanlu @DimedS, I've tried playing around with switching the windowing mode programatically, but its a lot more complex than I anticipated I think @astrojuanlu initial suggestion of inserting it all into one is easiest. I have a PR that checks if the user is running Jupyter Notebook >=7.2.0 and runs the output into 1 cell otherwise it uses the original approach. Let me know your thoughts.

@astrojuanlu
Copy link
Member

Thanks for investigating! I'd even go as far as saying that we make this consistent for all notebook versions, to make it even simpler for us.

@SajidAlamQB SajidAlamQB moved this from In Progress to In Review in Kedro 🔶 Mar 4, 2025
@SajidAlamQB SajidAlamQB linked a pull request Mar 4, 2025 that will close this issue
7 tasks
@github-project-automation github-project-automation bot moved this from In Review to Done in Kedro 🔶 Mar 6, 2025
@jtpio
Copy link

jtpio commented Mar 6, 2025

Coming here from jtpio/ipylab#136.

Interesting to see how the full windowing mode changes the behavior. Normally ipylab should simply execute JupyterLab commands, and commands should normally have the same effect for all windowing mode.

So it may still be something to investigate in ipylab I guess? But #4540 seems to have found a solution without the use of ipylab, which is also good!

@astrojuanlu
Copy link
Member

Hey @jtpio , thanks for chiming in! Indeed, @SajidAlamQB 's investigations point to a bad interaction between ipylab and the windowing mode. We did find a workaround but slightly changing how our command works, but if we can help in any way, let us know!

@jtpio
Copy link

jtpio commented Mar 7, 2025

Sure, thanks!

I guess this can be investigated as part of jtpio/ipylab#136. Activity on ipylab has been quite low lately, but will hopefully be able to put a few cycles into it at some point soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Bug Report 🐞 Bug that needs to be fixed
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants