marimo-team / marimo

A reactive notebook for Python — run reproducible experiments, execute as a script, deploy as an app, and version with git.
https://marimo.io
Apache License 2.0
6.72k stars 237 forks source link

[Windows] mpl.interactive does not work #273

Open akshayka opened 11 months ago

akshayka commented 11 months ago

This issue was being discussed in #260, but is separate from the original issue in that thread. So creating a new issue to track.

A user said that on the latest version of chrome, marimo 0.1.39, the only plot that shows up for the below code is seaborn. I am unable to repro.

import marimo

__generated_with = "0.1.38"
app = marimo.App()

@app.cell
def __(exp):
    exp
    return

@app.cell
def __(altp, exp, mo, mplplot, plotlyplot, snsplot):
    mo.vstack(
        [
            mo.md(f"$$y=x^{exp.value}$$"),
            mo.tabs(
                {
                    "altair": mo.vstack([altp, altp.value]),
                    "plotly": plotlyplot(exp.value),
                    "seaborn": snsplot(exp.value),
                    "matplotlib": mplplot(exp.value),
                }
            ),
        ]
    )
    return

@app.cell
def __():
    import marimo as mo
    from plotly import express as px
    import altair as alt
    import numpy as np
    import pandas as pd
    import seaborn as sns
    from matplotlib import pyplot as plt
    from functools import cache
    return alt, cache, mo, np, pd, plt, px, sns

@app.cell
def __(mo):
    exp = mo.ui.slider(1, 8, 1, label="exp: ")
    return exp,

@app.cell
def __(cache, np):
    @cache
    def prep_xy(exp):
        x = np.linspace(0, 5, 100)
        return x, x**exp
    return prep_xy,

@app.cell
def __(alt, cache, mo, pd, prep_xy):
    @cache
    def altplot(exp):
        x, y = prep_xy(exp)
        data = pd.DataFrame({"x": x, "y": y})
        return mo.ui.altair_chart(
            alt.Chart(data).mark_line().encode(x="x", y="y"),
            chart_selection="interval",
        )
    return altplot,

@app.cell
def __(cache, prep_xy, px):
    @cache
    def plotlyplot(exp):
        x, y = prep_xy(exp)
        return px.line(x=x, y=y)
    return plotlyplot,

@app.cell
def __(cache, prep_xy, sns):
    @cache
    def snsplot(exp):
        x, y = prep_xy(exp)
        return sns.lineplot(x=x, y=y)
    return snsplot,

@app.cell
def __(cache, mo, plt, prep_xy):
    @cache
    def mplplot(exp):
        x, y = prep_xy(exp)
        plt.plot(x, y)
        return mo.mpl.interactive(plt.gcf())
    return mplplot,

@app.cell
def __(altplot, exp):
    altp = altplot(exp.value)
    return altp,

if __name__ == "__main__":
    app.run()
akshayka commented 11 months ago

@JVSigaud, I can help you debug.

Can you do the following?

  1. Let me know what operating system you're using.
  2. Upload a screenshot of the output of the cell rendering the plots.
  3. If possible, run pip freeze > requirements.txt and upload requirements.txt here.

Thanks!

JVSigaud commented 11 months ago

image image image image Im using windows 11.

requirements.txt

akshayka commented 11 months ago

Thanks, this is very helpful. Likely this is a Windows-specific bug in our backend. I don't have immediate access to Windows right now, but should be able to get my hands on a Windows machine within a couple days.

@JVSigaud , I'll write back when I have a fix.

@mscolnick, FYI:

akshayka commented 11 months ago

@JVSigaud, FYI we just published marimo version 0.1.40, which should fix altair and plotly plots. We still need to fix mpl.interactive on Windows.

bjmcculloch commented 8 months ago

I ran into this, as well. In my case, I'm running marimo in WSL and viewing from a Windows browser on the same machine. The issue appears to be that the URL specified in the <iframe> element is not well-formed. In my case, I saw <iframe src="http://[::]:50751/" width="100%" height="550px">. If I simply edit the URL to http://localhost:50751/, then everything works as expected. I did a cursory search of the codebase, but it was not obvious to me where the URL generation happens.

Edit: I think the code is near line 269 in marimo/_plugins/stateless/mpl/_mpl.py

bjmcculloch commented 8 months ago

I did some testing with tornado.netutil.bind_sockets() in the WSL instance, and it indeed returns the IPv6 "all" address of ::, which the marimo code wraps in square braces to give [::].

>>> sockets = tornado.netutil.bind_sockets(0, "")
>>> sockets
[<socket.socket fd=4, family=2, type=1, proto=6, laddr=('0.0.0.0', 51128)>, <socket.socket fd=5, family=10, type=1, proto=6, laddr=('::', 51128, 0, 0)>]

So, the issue is that tornado is returning a bound address (0.0.0.0 and :: for IPv4 and IPv6, respectively) which is not the right way to reach localhost (e.g. 127.0.0.1 and ::1 for IPv4 and IPv6, respectively). I verified that editing the host in the iframe's URL (using the browser's inspector tool) to be [::1] instead of [::] also solves the issue.

I'm not a security expert, but it's probably not great to bind against all addresses. If it were up to me, I'd probably just bind to a loopback address and put that in the <iframe> src tag (though I'm sure that may break some particular workflow).

mscolnick commented 8 months ago

@bjmcculloch - we moved matplotlib interactive websockets to starlette as well as improved some performance there.

could you give it another try and see if it works now after that refactor?

bjmcculloch commented 8 months ago

@mscolnick Somewhat. The example in the first post of this issue now works as expected when initially run.

However, if you then edit and re-evaluate the cell containing the mo.tabs element, then the "matplotlib" tab then <div style="display: inline-block"> element grows vertically forever without the plot ever rendering.

This testing was conducted with the most recent checkout of main branch (commit 92abd0c4bea84a83814b1f04b155624c24c6f502) on Chrome v121 / Windows 10.

mscolnick commented 8 months ago

@bjmcculloch - it looks like its due to the @cache annotation on the function for mo.mpl.interactive. we likely tear down the mpl server, but the cache keeps the same output.

there is no reason to use @cache on marimo components (you can cache the args, but no need to cache the components)

bjmcculloch commented 8 months ago

@mscolnick I can confirm that upon removing @cache everything works as expected. Thank you very much!

wasimsandhu commented 1 month ago

@JVSigaud Hi! Just checking, is this defect still reproducible for you?