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

fix(display): Update display import #1709

Merged
merged 4 commits into from
Mar 4, 2025

Conversation

walterdiaza
Copy link
Contributor

@walterdiaza walterdiaza commented Mar 1, 2025

Fix display import for IPython 9.0.0

Description

This PR primarily focuses on updating the display import to support IPython 9.0.0. Previously, we were using:

from IPython.core.display import display

However, this is no longer compatible with IPython 9.0.0. We now import display from:

from IPython.display import display

It's important to note that this module has been used in earlier versions of IPython as well, so this change does not break compatibility with previous releases.

Overall, these updates ensure full compatibility with the latest version of IPython while maintaining support for older versions . All modifications have been tested locally, and the expected behavior remains unchanged.

I appreciate your review and welcome any feedback or suggestions.
Fixes #1708

@walterdiaza walterdiaza changed the title Fix display import for IPython 9.0.0 and adjust minor typing issues fix(display): Update display import and minor typing adjustments Mar 1, 2025
@walterdiaza walterdiaza changed the title fix(display): Update display import and minor typing adjustments fix(display): Update display import Mar 1, 2025
@fabclmnt fabclmnt self-requested a review March 3, 2025 21:37
@fabclmnt
Copy link
Collaborator

fabclmnt commented Mar 3, 2025

Hi @walterdiaza ,

thank you for your contribution! It seems like the unit testing is failing, as they need an update to be aligned with the changes that you've made.

Nevertheless, I do have a question - In case users have an older version of IPython installed, would it still work?

Copy link
Collaborator

@fabclmnt fabclmnt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests update needed

@ssiegel ssiegel mentioned this pull request Mar 3, 2025
@ssiegel
Copy link
Contributor

ssiegel commented Mar 4, 2025

Changes to the test notebooks (meteorites.ipynb, titanic.ipynb) are not necessary, they are causing the failures. Reverting them to their original state fixes the issue. And yes, it works for older versions of IPython, the tests running on python<3.11 cover that.

@fabclmnt
Copy link
Collaborator

fabclmnt commented Mar 4, 2025

Changes to the test notebooks (meteorites.ipynb, titanic.ipynb) are not necessary, they are causing the failures. Reverting them to their original state fixes the issue. And yes, it works for older versions of IPython, the tests running on python<3.11 cover that.

The tests need to be updated.

Changes to the test notebooks (meteorites.ipynb, titanic.ipynb) are not necessary, they are causing the failures. Reverting them to their original state fixes the issue. And yes, it works for older versions of IPython, the tests running on python<3.11 cover that.

Reverted to the original state or otherwise, the current PR needs to be updated so that the tests don't fail.

@walterdiaza can you please perform the changes?
Otherwise, we will be forced to move forward without your changes, as we expect to have the release launched by tomorrow and I would love to have your contribution completed.

@fabclmnt fabclmnt merged commit f593bb7 into ydataai:develop Mar 4, 2025
6 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug Report: Error in to_notebook_iframe() – ImportError with IPython 9.0.0
3 participants