justpy-org / justpy

An object oriented high-level Python Web Framework that requires no frontend programming
https://justpy.io
Apache License 2.0
1.22k stars 96 forks source link

AgGrid cannot update with a np.nan value #571

Closed getzze closed 2 years ago

getzze commented 2 years ago

I am reporting a bug I got with nicegui: https://github.com/zauberzeug/nicegui/issues/126

I'm copying the code to reproduce the error in justpy

#!/usr/bin/env python3
import justpy as jp
import numpy as np

options = {
    'columnDefs': [
        {'headerName': 'Name', 'field': 'name'},
        {'headerName': 'Age', 'field': 'age'},
    ],
    'rowData': [
        {'name': 'Alice', 'age': 18},
        {'name': 'Bob', 'age': 21},
        {'name': 'Carol', 'age': 42},
    ],
}

def grid_test():
    wp = jp.WebPage()
    grid = jp.AgGrid(a=wp, options=options, classes='max-h-60')

    def update(self, msg):
        grid.options.rowData[0].age += 1
        grid.options.rowData[1].age = np.nan

    jp.Button(text='Update', click=update, a=wp)
    return wp

jp.justpy(grid_test)

It seems to be a problem of translating the special value np.nan to Javascript. Probably the same problem would arrive with other special values: np.inf, pd.NA, ...

It was hard to find this bug because it was happening when importing a pandas DataFrame. In this case, it was solved by calling df.fillna("") before importing the DataFrame.

WolfgangFahl commented 2 years ago

for python None values the problem is well known. Justpy can only handle data with proper JSON representations and tries to fix up things on start. Which version did you use when trying - the fix up code below should have handled your case on loading. Setting a value in the way your example does to np.nan is calling for trouble with the current approach. IMHO the idea to try to display pandas dataframes directly as ag-grid cells is flawed in principle since there is then no separation of concerns between the data and how the data is viewed. Of course there are transformations necessary from data to view and vice versa and currently justpy doesn't support such an approach. Since there have been multiple issues and dicussions on this already we might need to come up with a generally better approach how to handle the different usecase instead of patching up the current code.

       # Change NaN and similar to None for JSON compatibility
        self.options.rowData = (
            df.replace([np.inf, -np.inf], [sys.float_info.max, -sys.float_info.max])
            .where(pd.notnull(df), None)
            .to_dict("records")
        )
WolfgangFahl commented 2 years ago

a pull request is most welcome which will make you a contributor!

WolfgangFahl commented 2 years ago

For this issues there is now example code available at https://github.com/justpy-org/justpy/blob/master/examples/issues/issue_571_aggrid_nan.py and as a live demo at: http://justpy-demo.herokuapp.com/demo/issue_571_aggrid_nan

There is now a load_lod function available and if you'd like to see automatic conversion of None / NaN values in this function please file a separate issue / change request. For the time being you might want to avoid NaN and None values when manipulation aggrid content