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.27k stars 202 forks source link

notebook.run() makes pytest fail if it contains mo.mpl.interactive(plt.gcf()) #1888

Closed ggggggggg closed 1 month ago

ggggggggg commented 1 month ago

Describe the bug

I ran a test with pytest of a notbook with mo.mpl.interactive(plt.gcf()), pytest prints that the tests all passed, but it never returns so the terminal is blocked and it will time out on CI.

Environment

{ "marimo": "0.7.11", "OS": "Windows", "OS Version": "10", "Processor": "Intel64 Family 6 Model 85 Stepping 4, GenuineIntel", "Python Version": "3.10.11", "Binaries": { "Browser": "126.0.6478.127", "Node": "--" }, "Requirements": { "click": "8.1.7", "importlib-resources": "missing", "jedi": "0.19.0", "markdown": "3.6", "pymdown-extensions": "10.8.1", "pygments": "2.16.1", "tomlkit": "0.12.5", "uvicorn": "0.30.1", "starlette": "0.37.2", "websockets": "12.0", "typing-extensions": "4.7.1", "ruff": "0.5.5" } }

also matplotlib and pytest info: matplotlib 3.7.2 matplotlib-inline 0.1.6 pytest 7.4.1 pytest-asyncio 0.21.1

Code to reproduce

in a file test_stuff.py

def test_example_marimo_ebit_off():
    from . import mwe_hang_marimo as notebook
    notebook.app.run()

in a file in the same directory and module

import marimo

__generated_with = "0.7.11"
app = marimo.App(width="medium")

@app.cell
def __():
    import marimo as mo
    import pylab as plt

    plt.plot(range(100))
    mo.mpl.interactive(plt.gcf())
    return mo, plt

if __name__ == "__main__":
    app.run()

then from the command line, in the base directory of the package, pytest

akshayka commented 1 month ago

Thanks for reporting. This was fixed in https://github.com/marimo-team/marimo/pull/1870, but we haven't released it yet.

I can release soon; the fix will be in 0.7.12.

akshayka commented 1 month ago

0.7.12 has been cut. It should be on PyPI within 30 minutes.

ggggggggg commented 1 month ago

The example here no longer hangs, but it now crashes with pytest, see the output below. python mwe_hang_marimo.py works fine, but running it from within pytest crashes.

================================================= test session starts =================================================
platform win32 -- Python 3.10.11, pytest-7.4.1, pluggy-1.3.0
rootdir: C:\Users\user\Desktop\python\moss
plugins: anyio-4.0.0, asyncio-0.21.1
asyncio: mode=strict
collected 3 items

moss\test_notebooks.py F.                                                                                        [ 66%]
moss\test_stuff.py .                                                                                             [100%]

====================================================== FAILURES =======================================================
________________________________________________ test_trivial_notebook ________________________________________________

    def test_trivial_notebook():
        from . import mwe_hang_marimo as notebook
>       notebook.app.run()

moss\test_notebooks.py:10:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
..\..\..\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\marimo\_ast\app.py:282: in run
    outputs, glbls = AppScriptRunner(InternalApp(self)).run()
..\..\..\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\marimo\_runtime\app\script_runner.py:111: in run
    raise e.__cause__ from None  # type: ignore
..\..\..\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\marimo\_runtime\executor.py:193: in execute_cell
    return eval(cell.last_expr, glbls)
C:\Users\user\AppData\Local\Temp\marimo_17720\__marimo__cell_Hbol__output.py:5: in <module>
    ???
..\..\..\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\marimo\_plugins\stateless\mpl\_mpl.py:265: in interactive
    return as_html(figure)
..\..\..\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\marimo\_output\formatting.py:270: in as_html
    return Html(data)
..\..\..\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\marimo\_output\hypertext.py:100: in __init__
    flat_text = flatten_string(self._text)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

text = None

    def flatten_string(text: str) -> str:
>       return "".join([line.strip() for line in text.split("\n")])
E       AttributeError: 'NoneType' object has no attribute 'split'

..\..\..\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\marimo\_output\utils.py:17: AttributeError
=============================================== short test summary info ===============================================
FAILED moss/test_notebooks.py::test_trivial_notebook - AttributeError: 'NoneType' object has no attribute 'split'
============================================= 1 failed, 2 passed in 5.54s =============================================
akshayka commented 1 month ago

@ggggggggg , thank you for reporting back.

I am unable to reproduce this on my machine using your MWE. But I maybe have a hunch what is happening.

Can you try something for me? Can you change mwe_hang_marimo to the following, and let me know if pytest is still failing?

import marimo

__generated_with = "0.7.11"
app = marimo.App(width="medium")

@app.cell
def __():
    import marimo as mo
    import pylab as plt

    plt.plot(range(100))
    mo.mpl.interactive(plt.gcf())
    return mo, plt

if __name__ == "__main__":
    app.run()
ggggggggg commented 1 month ago

Thanks for looking into it. Using the code you posted did not fix it, and I wasn't able to spot what you changed. But I did a bit more testing and have a few things that may help.

  1. I can reproduce the problem with this file alone
import marimo

__generated_with = "0.7.11"
app = marimo.App(width="medium")

@app.cell
def __():
    import marimo as mo
    import pylab as plt

    plt.plot(range(100))
    mo.mpl.interactive(plt.gcf())
    return mo, plt

def test_trivial_notebook():
    app.run()

if __name__ == "__main__":
    app.run()
  1. It works (aka does not fail) if I put it in an empty directory and run pytest.
  2. It fails with the same error as before if I put it in the same directory as before, which has like 15 other python files, is part of an installed editable package, and has some random other .txt files in it.

I have to run now, but when I get back I'm going to try deleting files one by one and see if I can switch it from failing to working by doing so.

akshayka commented 1 month ago

Oops, my mistake! I meant to replace pylab with matplotlib.pyplot:

import marimo

__generated_with = "0.7.11"
app = marimo.App(width="medium")

@app.cell
def __():
    import marimo as mo
    import matplotlib.pyplot as plt

    plt.plot(range(100))
    mo.mpl.interactive(plt.gcf())
    return mo, plt

if __name__ == "__main__":
    app.run()

Thanks for doing additional debugging. I will wait to hear back on what you find.

ggggggggg commented 1 month ago

That doesn't fix it, but I now have a minimized example. You need two files in the same folder, their full contents in this gist. It's the same file I posted above, plus __init__.py with the line import pylab as plt. Then you run pytest from that folder or a parent folder. This seems like the bug may not even be in marimo, but you'll know better than I.

akshayka commented 1 month ago

Thank you. That's very helpful. I'll work on a fix.