skrub-data / skrub

Prepping tables for machine learning
https://skrub-data.org/
BSD 3-Clause "New" or "Revised" License
1.23k stars 99 forks source link

Interaction between `TableReport` and `matplotlib` #1171

Open glemaitre opened 22 hours ago

glemaitre commented 22 hours ago

Given the following code blocks:

# %%
import skrub

# %%
from sklearn.datasets import load_iris
data, target = load_iris(return_X_y=True, as_frame=True)
TableReport(data)

# %%
data["sepal length (cm)"].plot.hist()

We expect (in a notebook or vs code) that running those cell will end-up showing the report and the histogram plot. However, the later cell requires a plt.show() to show the plot. This is not the behaviour when not patching the display.

Also it seems that if we intend a first plot, then the interaction with matplotlib does not happen:

# %%
import skrub

# %%
from sklearn.datasets import load_iris
data, target = load_iris(return_X_y=True, as_frame=True)

# %%
data["sepal length (cm)"].plot.hist()

# %%
skrub.TableReport(data)

# %%
data["sepal length (cm)"].plot.hist()

I'm wondering if this is something expected?

jeromedockes commented 20 hours ago

I'm wondering if this is something expected?

:joy: :sweat_smile:

thanks for reporting I'll look into it

jeromedockes commented 18 hours ago

@glemaitre so it seems using matplotlib.rc_context anywhere is enough to prevent the display of figures in a notebook. here is a reproducer without skrub could you check if it has the same effect for you?

import matplotlib
from matplotlib import pyplot as plt

with matplotlib.rc_context():
    plt.close(plt.figure())

# %%
plt.plot([1, 2], [1, 2])
jeromedockes commented 18 hours ago

matplotlib issue: https://github.com/matplotlib/matplotlib/issues/25041

jeromedockes commented 18 hours ago

also reported here: https://github.com/matplotlib/matplotlib/issues/26716

glemaitre commented 18 hours ago

Interesting. At least there is a work around.

jeromedockes commented 18 hours ago

the matplotlib context manager does not seem to do anything more complex than updating matplotlib.rcParams so we can do it ourselves, and by updating only the key we need it seems to fix the issue: #1172

let me know if it works on your side :)