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

matplotlib: savefig() produces TypeError: Invalid argument type in ToBigInt operation #8

Open
ashley-hh opened this issue Nov 2, 2021 · 11 comments

Comments

@ashley-hh
Copy link

ashley-hh commented Nov 2, 2021

Hello fellow pyodide users and mods!! Thank you for your hard work, this is an awesome project and I've been using it heavily!! Most recently, I've been trying to plot clustermaps from seaborn. This works amazingly on Chrome and somewhat well on Firefox, but not at all on Safari. This is the error that I've been getting when I call savefig() from the matplotlib library:

Screen Shot 2021-11-02 at 2 22 07 AM

Chasing down the most recent stack call, it's this function:
Screen Shot 2021-11-02 at 2 23 26 AM

Before I go even further down the rabbit hole to bisect this error, has anyone already encountered this and know how to get around it? Is this even the right place to ask this question? This is my first experience with cross-platform software, so any advice would be greatly appreciated. Thanks in advance, and hope y'all have a lovely day!!

@ashley-hh ashley-hh changed the title Matplotlib and Seaborn: savefig() produces TypeError: Invalid argument type in ToBigInt operation matplotlib and seaborn: savefig() produces TypeError: Invalid argument type in ToBigInt operation Nov 2, 2021
@ryanking13
Copy link
Member

Thanks for the report, could you provide a minimal code example to reproduce this error?

@hoodmane
Copy link
Member

hoodmane commented Nov 2, 2021

We still don't fully support Safari: we don't run Safari tests in the continuous integration and I don't think any of the core devs use macs so we don't test things locally in Safari. I frequently do manual experiments in both Firefox and Chrome but not in Safari. So it may be a long time before issues like this are addressed.

@rth
Copy link
Member

rth commented Nov 2, 2021

use macs so we don't test things locally in Safari

Actually once pyodide/pyodide#1912 is resolved, we could potentially run Safari in CI. It adds more maintenance work, but then we do have recurrent requests to for better Safari support as well...

@ashley-hh
Copy link
Author

ashley-hh commented Nov 2, 2021

Thanks for the report, could you provide a minimal code example to reproduce this error?

Thank you for your prompt response!! I believe this should do it:

Screen Shot 2021-11-02 at 3 01 06 PM

I'm using Version 14.1.2 of Safari.

@rth
Copy link
Member

rth commented Nov 2, 2021

Thanks @ashley-hh ! Could you please copy paste it as a code snippet so that we wouldn't have to re-type it manually?

@ashley-hh
Copy link
Author

ashley-hh commented Nov 2, 2021

Oh, of course! My bad -- here you go!

const script = `
import micropip
import numpy as np
import os
os.environ['MPLBACKEND'] = 'AGG'
import io, base64
import sys
sys.setrecursionlimit(1000)
await micropip.install('seaborn==0.9.0')
import seaborn as sns

data = np.asarray([[1, 0, .5], [1, 0, .5], [1, 0, .5]])
fig = sns.clustermap(data)

buf = io.BytesIO()
fig.savefig(buf, format='png')`

await self.pyodide.loadPackagesFromImports(script);
const res = await self.pyodide.runPythonAsync(script);

@ashley-hh ashley-hh changed the title matplotlib and seaborn: savefig() produces TypeError: Invalid argument type in ToBigInt operation matplotlib: savefig() produces TypeError: Invalid argument type in ToBigInt operation Nov 2, 2021
@rth
Copy link
Member

rth commented Nov 3, 2021

Thanks! I can reproduce on Safari 14.1. As far as I can tell the error happens when calling sns.clustermap, even without the savefig part.

BTW, in Safari 13.1 I get,

Error: WebAssembly function with an i64 argument can't be called from JavaScript (
      evaluating `dynCall("viiii", index, [a1, a2, a3, a4])`

The error is likely different because 14.1 got BigInt support. So far not much ideas of how to fix it, but clearly some argument is not being converted while it should be, since we are building without WASM_BIGINT as far as I know.

@hoodmane
Copy link
Member

hoodmane commented Nov 3, 2021

The flag WASM_BIGINT says that BigUint64Array and BigInt64Array exist. Emscripten always assumes that BigInt exists.

@rth
Copy link
Member

rth commented Nov 4, 2021

@hoodmane
Copy link
Member

hoodmane commented Nov 4, 2021

I think so. For a function with an i64 argument, dynCall always gets a BigInt. But to load or store BigInt into the wasm memory without BigUint64Array requires some bitwise operations. See here in my libffi port:

https://github.com/hoodmane/libffi-emscripten/blob/1fd79801595a43e26b65b59030a9040ba124ee24/src/wasm32/ffi.c#L51]

Both with and without WASM_BIGINT, we are expected to handle BigInt but without it HEAPU64 doesn't exist. Without BigInt support at all, it's just impossible to call functions with an i64 argument from Javascript as the error message says.

@hoodmane
Copy link
Member

hoodmane commented Nov 4, 2021

It's funny, grepping around the emscripten source, I don't find much evidence for my claim that WASM_BIGINT works this way, but the libffi tests pass when built without WASM_BIGINT so that makes me think it's probably true anyways.

@rth rth transferred this issue from pyodide/pyodide Sep 10, 2022
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

No branches or pull requests

4 participants