vega / vl-convert

Utilities for converting Vega-Lite specs from the command line and Python
BSD 3-Clause "New" or "Revised" License
106 stars 14 forks source link

Saving fails when called from pythonw #188

Open tonyroberts opened 1 month ago

tonyroberts commented 1 month ago

Saving a chart from Python as png or svg fails when the process has no stdin. This can happen when Python is embedded in another application, or when running a script using pythonw instead of python.

OS: Windows Python version: 3.11 Altair version: 5.4.1 vl-convert version: 1.6.1

To reproduce this issue run the following script using pythonw (not python) on Windows. The logging is necessary since pythonw won't log to stdout/stderr:

import logging
logging.basicConfig(filename="vl-test.log", level=logging.INFO)
log = logging.getLogger(__file__)

try:
    import altair as alt
    import pandas as pd

    df = pd.DataFrame({"X": [1,2,3], "Y": [1,2,3]})
    chart = alt.Chart(df).mark_line().encode(x="X", y="Y")

    chart.save("vl-chart.png")
    log.info("Chart saved")
except:
    log.error("Error saving chart", exc_info=True)
    raise

This fails with the following error:

Traceback (most recent call last):
  File "vl-test.py", line 17, in <module>
    chart.save("vl-chart.png")
  File "C:\Users\tony\miniconda3\envs\py311\Lib\site-packages\altair\vegalite\v5\api.py", line 2092, in save
    save(**kwds)
  File "C:\Users\tony\miniconda3\envs\py311\Lib\site-packages\altair\utils\save.py", line 224, in save
    perform_save()
  File "C:\Users\tony\miniconda3\envs\py311\Lib\site-packages\altair\utils\save.py", line 175, in perform_save
    mb_png = spec_to_mimebundle(
             ^^^^^^^^^^^^^^^^^^^
  File "C:\Users\tony\miniconda3\envs\py311\Lib\site-packages\altair\utils\mimebundle.py", line 134, in spec_to_mimebundle
    return _spec_to_mimebundle_with_engine(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\tony\miniconda3\envs\py311\Lib\site-packages\altair\utils\mimebundle.py", line 244, in _spec_to_mimebundle_with_engine
    png = vlc.vegalite_to_png(
          ^^^^^^^^^^^^^^^^^^^^
ValueError: Vega-Lite to PNG conversion failed:
Failed to retrieve conversion result: oneshot canceled

The error is because of the worker thread failing with this error:

ERROR : thread '<unnamed>' panicked at C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\deno_runtime-0.166.0\worker.rs:701:7:
ERROR : Bootstrap exception: Error: null or invalid handle
ERROR :     at guessHandleType (ext:deno_node/internal_binding/util.ts:38:16)
ERROR :     at _guessStdinType (ext:deno_node/_process/streams.mjs:105:10)
ERROR :     at initStdin (ext:deno_node/_process/streams.mjs:132:38)
ERROR :     at Object.internals.__bootstrapNodeProcess (node:process:653:22)
ERROR :     at initialize (ext:deno_node/02_init.js:34:15)
ERROR :     at bootstrapMainRuntime (ext:runtime_main/js/99_main.js:873:7)

Possibly related to #181?

tonyroberts commented 1 month ago

This used to work and I've narrowed down when it stopped working to between vl-convert-python 1.1.0 and 1.2.0.

vl-convert-python 1.1.0 works, 1.2.0 fails.

jonmmease commented 1 month ago

Thanks for the report @tonyroberts. The differnece between 1.1.0 and 1.2.0 is almost certainly the update from deno 1.37.2 to 1.38.4. Deno is due for another update, so we can see if that makes a difference. I wonder if this is reproducible outside of windows (which would make it easier for me to debug).

Are you in a situation where you have a requirement to use pythonw?

tonyroberts commented 1 month ago

Hi @jonmmease,

Yes I think you're right about it being a difference in the Deno version.

I don't have a non-Windows environment set up I can easily test this on right now... Actually the problem is not only using pythonw, that was just the easiest way to reproduce the issue. The actual problem for me is when calling this code from Python embedded in another Windows application (Excel) that doesn't have a console (so no stdin).

I was just using the pre-built binaries from pypi, but I can try building from source with a newer Deno version and see if that helps. If you could please leave this open for now I will update you once I've had a chance to try that.

Thanks!

jonmmease commented 1 month ago

Thanks for the additional context, and it is helpful to know that pythonw reproduces the issue, as that's certainly easier for me to play with than embedding.

Let me know what you find (or run into) when updating deno. Some past updates have't required any code changes in vl-convert, and others have required some rework, so I don't know how hairy this update will be.

If the Deno update doesn't help, or if you run into issues with the update, another thing that would be helpful for me would be a PR that adds a new windows GitHub Action to https://github.com/vega/vl-convert/blob/main/.github/workflows/CI.yml that uses pythonw to reproduce the error.

jonmmease commented 4 weeks ago

vl-convert 1.7.0 uses the latest version of Deno, so it's worth another try!

tonyroberts commented 4 weeks ago

I just tried with the latest version and got the same error, unfortunately :( I will look at writing a test and raise a PR.

tonyroberts commented 4 weeks ago

I've submitted PR #197 that shows the issue in the Windows test job. It fails with the same error as reported

vl-convert-python\tests\test_specs.py:217: AssertionError
=========================== short test summary info ===========================
FAILED vl-convert-python\tests\test_specs.py::test_svg_pythonw[True-circle_binned] - assert 'Traceback (m...ot canceled\n' == 'ok'

  - ok
  + Traceback (most recent call last):
  +   File "D:\a\vl-convert\vl-convert\vl-convert-python\tests\test_specs.py", line 191, in pythonw_func
  +     test_svg(name, as_dict)
  +   File "D:\a\vl-convert\vl-convert\vl-convert-python\tests\test_specs.py", line 177, in test_svg
  +     vg_spec = vlc.vegalite_to_vega(vl_spec, vl_version=vl_version)
  +               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  + ValueError: Vega-Lite to Vega conversion failed:
  + Failed to retrieve conversion result: oneshot canceled
jonmmease commented 4 weeks ago

Thanks, that's very helpful!