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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 ipydatagrid>=1.3 doesn't support integer pandas column names #512

Closed
jgunstone opened this issue Apr 30, 2024 · 2 comments 路 Fixed by #517
Closed

馃悰 ipydatagrid>=1.3 doesn't support integer pandas column names #512

jgunstone opened this issue Apr 30, 2024 · 2 comments 路 Fixed by #517

Comments

@jgunstone
Copy link
Contributor

jgunstone commented Apr 30, 2024

... as unwanted column created with string name matching the integer on set_cell_value

I'm just trying to update ipyautoui to ipydatagrid>1.3.1 and came across this tricky little issue:

Describe the bug

a new column with the string name of the edited column is created. This only happens when retrieving the data in a new cell.
image

To Reproduce

from ipydatagrid import DataGrid
import pandas as pd

df = pd.DataFrame({0:{"a":"a", "b": 1}, 1:{"a":"abba", "b": 2}})
display(df)
gr = DataGrid(df)
assert list(gr.data.columns) == [0, 1]
column_name, primary_key_value, new_value = 1, 'a', "my new string"
gr.set_cell_value(column_name, primary_key_value, new_value)
display(gr.data)
assert list(gr._data["data"]) == ["key", 0, 1, "ipydguuid"]
display(gr._data["data"].columns)
gr._data["data"]

Expected behavior
Behaves as expected for non-integer column names

image

Environment (please complete the following information):

  • Operating System and Version: [e.g. Ubuntu Linux 22.04]
  • Browser Chrome
  • ipydatagrid=1.3.1
  • jupyterlab=4.1.8

@martinRenou - maybe you could help?

@jgunstone
Copy link
Contributor Author

feels like a weird jupyter related bug so I just tried with jupyterlab=4.2
jupyterlab/jupyterlab#15801

and it worked!
image

looks like that is slated for this week so I'll just hang tight until its out and try again

@jgunstone
Copy link
Contributor Author

just tried with the jupyterlab==4.2 and unfortunately I'm getting the same original error

image

if I make the col names strings not numbers the bad behaviour stops, i.e.
image

it seems as though on the JS side of ipydatagrid (or in lumino?) the integer col names are being coerced to strings and an additional column is added.

looking through the js side of things:

updateRowValue(options: ViewBasedJSONModel.IUpdateRowValuesOptions): void {
// Create new row and add it to new dataset
for (const columnIndex of Array(options.value.length).keys()) {
const columnName = this.metadata('body', 0, columnIndex)['name'];
this._dataset.data[columnName][options.row] = options.value[columnIndex];
}
// We need to rerun the transforms, as the changed cells may change the order
// or visibility of other rows
this.currentView = this._transformState.createView(this._dataset);
}

metadata(
region: DataModel.CellRegion,
row: number,
column: number,
): DataModel.Metadata {
const md = this.currentView.metadata(region, row, column);
if (region == 'body') {
md.row = row;
md.column = column;
md.data = (row: number, column: string) => {
const columnIndex = this.columnNameToIndex(column.toString());
return this.data('body', row, columnIndex);
};
}
return md;

https://github.com/jupyterlab/lumino/blob/43ae67c5cfe90b04acff3bbdd12cce9bb7a879bf/packages/datagrid/src/jsonmodel.ts#L150-L158

it appears from the above that there is a requirement to use a string for the name (though this wasn't previously enforced).
FYI the referenced frictionless schema spec doesn't explicitly require the column name to be a string.

Does ipydatagrid require the pandas DataFrame to have string col names? should it?

@jgunstone jgunstone changed the title 馃悰 unwanted column created on set_cell_value 馃悰 ipydatagrid>=1.3 doesn't support integer pandas column names May 8, 2024
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

Successfully merging a pull request may close this issue.

1 participant