fix(PandasDataFrameItem): complex objects are persisted

koaning commented 3 weeks ago

Describe the bug

I tried running skore from Jupyter with this line:

! python -m skore launch "alpha-demo"

When I looked at the browser I saw this:

CleanShot 2024-10-26 at 11 18 02

The notebook contained this traceback.

Running skore UI from 'alpha-demo' at URL http://localhost:22140/
ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 324, in jsonable_encoder
    data = dict(obj)
TypeError: cannot convert dictionary update sequence element #0 to a sequence

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 329, in jsonable_encoder
    data = vars(obj)
TypeError: vars() argument must have __dict__ attribute

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/uvicorn/protocols/http/h11_impl.py", line 406, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/uvicorn/middleware/proxy_headers.py", line 60, in __call__
    return await self.app(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/applications.py", line 1054, in __call__
    await super().__call__(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/applications.py", line 113, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/middleware/errors.py", line 187, in __call__
    raise exc
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/middleware/errors.py", line 165, in __call__
    await self.app(scope, receive, _send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/middleware/cors.py", line 85, in __call__
    await self.app(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/middleware/exceptions.py", line 62, in __call__
    await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    raise exc
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/_exception_handler.py", line 42, in wrapped_app
    await app(scope, receive, sender)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/routing.py", line 715, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/routing.py", line 735, in app
    await route.handle(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/routing.py", line 288, in handle
    await self.app(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/routing.py", line 76, in app
    await wrap_app_handling_exceptions(app, request)(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    raise exc
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/_exception_handler.py", line 42, in wrapped_app
    await app(scope, receive, sender)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/routing.py", line 73, in app
    response = await f(request)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/routing.py", line 327, in app
    content = await serialize_response(
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/routing.py", line 201, in serialize_response
    return jsonable_encoder(response_content)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 245, in jsonable_encoder
    return jsonable_encoder(
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 289, in jsonable_encoder
    encoded_value = jsonable_encoder(
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 289, in jsonable_encoder
    encoded_value = jsonable_encoder(
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 289, in jsonable_encoder
    encoded_value = jsonable_encoder(
  [Previous line repeated 1 more time]
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 303, in jsonable_encoder
    jsonable_encoder(
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 303, in jsonable_encoder
    jsonable_encoder(
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 332, in jsonable_encoder
    raise ValueError(errors) from e
ValueError: [TypeError('cannot convert dictionary update sequence element #0 to a sequence'), TypeError('vars() argument must have __dict__ attribute')]

Steps/Code to Reproduce

Figured that I might try and paste the markdown below of the notebook.

from sklearn.datasets import make_regression
from sklearn.model_selection import GridSearchCV
from sklearn.linear_model import Ridge
import numpy as np

X, y = make_regression(n_samples=1000, noise=100)

cv = GridSearchCV(
    Ridge(), 
    param_grid={"alpha": np.logspace(-3, 5, 100)},
    scoring="neg_mean_squared_error"
)

cv.fit(X, y)

import polars as pl 
import altair as alt

df = pl.DataFrame(cv.cv_results_).with_columns(mse=-pl.col("mean_test_score"))

chart = alt.Chart(df).mark_line().encode(
    x=alt.X("param_alpha").scale(type="log"), 
    y='mse'
).properties(
    title="mean squared error vs. alpha"
)

from skore import

! python -m skore create alpha-demo

Project file '/Users/vincent/Development/probabl/alpha-demo.skore' was successfully created.

chart

from skore import load
import pandas as pd

skore_proj = load("alpha-demo")
skore_proj.put("cv", cv)
skore_proj.put("df", pd.DataFrame(df))
skore_proj.put("chart", chart)

! python -m skore launch "alpha-demo"

Running skore UI from 'alpha-demo' at URL http://localhost:22140
[31mERROR[0m:    Exception in ASGI application
Traceback (most recent call last):
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 324, in jsonable_encoder
    data = dict(obj)
TypeError: cannot convert dictionary update sequence element #0 to a sequence

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 329, in jsonable_encoder
    data = vars(obj)
TypeError: vars() argument must have __dict__ attribute

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/uvicorn/protocols/http/h11_impl.py", line 406, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/uvicorn/middleware/proxy_headers.py", line 60, in __call__
    return await self.app(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/applications.py", line 1054, in __call__
    await super().__call__(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/applications.py", line 113, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/middleware/errors.py", line 187, in __call__
    raise exc
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/middleware/errors.py", line 165, in __call__
    await self.app(scope, receive, _send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/middleware/cors.py", line 85, in __call__
    await self.app(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/middleware/exceptions.py", line 62, in __call__
    await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    raise exc
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/_exception_handler.py", line 42, in wrapped_app
    await app(scope, receive, sender)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/routing.py", line 715, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/routing.py", line 735, in app
    await route.handle(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/routing.py", line 288, in handle
    await self.app(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/routing.py", line 76, in app
    await wrap_app_handling_exceptions(app, request)(scope, receive, send)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    raise exc
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/_exception_handler.py", line 42, in wrapped_app
    await app(scope, receive, sender)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/starlette/routing.py", line 73, in app
    response = await f(request)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/routing.py", line 327, in app
    content = await serialize_response(
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/routing.py", line 201, in serialize_response
    return jsonable_encoder(response_content)
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 245, in jsonable_encoder
    return jsonable_encoder(
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 289, in jsonable_encoder
    encoded_value = jsonable_encoder(
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 289, in jsonable_encoder
    encoded_value = jsonable_encoder(
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 289, in jsonable_encoder
    encoded_value = jsonable_encoder(
  [Previous line repeated 1 more time]
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 303, in jsonable_encoder
    jsonable_encoder(
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 303, in jsonable_encoder
    jsonable_encoder(
  File "/Users/vincent/Development/probabl/.venv/lib/python3.10/site-packages/fastapi/encoders.py", line 332, in jsonable_encoder
    raise ValueError(errors) from e
ValueError: [TypeError('cannot convert dictionary update sequence element #0 to a sequence'), TypeError('vars() argument must have __dict__ attribute')]

Expected Behavior

I would not have expected an error at all, but a more helpful error message would have also been grand.

Actual Behavior

see above.

Environment

System:
    python: 3.10.14 (main, Aug 14 2024, 05:14:46) [Clang 18.1.8 ]
executable: /Users/vincent/Development/probabl/.venv/bin/python3
   machine: macOS-13.4.1-arm64-arm-64bit

Python dependencies:
        skore: 0.2.0
          pip: None
   setuptools: 75.1.0
    diskcache: 5.6.3
      fastapi: 0.115.3
 plotly<6,>=5: None
         rich: 13.9.2
        skops: 0.10.0
      uvicorn: 0.32.0

thomass-dev commented 3 weeks ago

@koaning to quickly fix your notebook, please use df_polars.to_pandas() to convert dataframe from polars to pandas instead of pd.Dataframe(df_polars), it will convert automatically numpy.array to dict/list.

I'm fixing on our side.

edit* to be more precise and resume, there are 2 problems:

the dataframe including complex objects (numpy-array here) is not actually well serialized in the back-end,
the index are lost during conversion from polars to pandas.

augustebaum commented 3 weeks ago

The problem seems to be that in your dataframe, the sixth column "params" is a struct, which gets converted to a numpy array when converting to pandas. So far we've assumed that values in dataframes are simple (ints, strings); we're working on a fix now!

koaning commented 3 weeks ago

Ah yeah, that is accurate. The cv results also put JSON stuff in there. I suppose this makes sense, but maybe for the future it would be nice if part of the traceback would still be shown here.

thomass-dev commented 3 weeks ago

@koaning the fix is under review #621 .

Have you tried this:

to quickly fix your notebook, please use df_polars.to_pandas() to convert dataframe from polars to pandas instead of pd.Dataframe(df_polars), it will convert automatically numpy.array to dict/list.

koaning commented 3 weeks ago

I can confirm that this indeed works.

I wonder, maybe just as a safety mechanism, would it make sense to check if an item can render during the .put call? The reason I bring this up is because it feels really brittle that a single .put call can bring the entire frontend experience down. If a colleague does this by accident, I may no longer be able to render any of the previous views. Not to mention the fact it would be very hard to figure out where the issue is.

Debugging stuff like this feels hard enough that we may want to be really defensive about it.

thomass-dev commented 3 weeks ago

I think its mainly a backend responsability to assure that all added items can be exported. But you're true, we shoudn't block the whole frontend experience when an item can't be exported. I've added an new issue: #628 .

probabl-ai / skore