From 2ca7ef4ca41f30e980380dce2e6919f92409637b Mon Sep 17 00:00:00 2001 From: David Kircos Date: Tue, 21 Feb 2023 21:22:13 -0700 Subject: [PATCH 1/3] add basis for test --- .../controller/tests/cell_evaluation.test.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/grid/controller/tests/cell_evaluation.test.ts b/src/grid/controller/tests/cell_evaluation.test.ts index b53f585862..4b9d620b1e 100644 --- a/src/grid/controller/tests/cell_evaluation.test.ts +++ b/src/grid/controller/tests/cell_evaluation.test.ts @@ -257,3 +257,29 @@ test('SheetController - test array formula', async () => { expect(cell.type).toEqual('COMPUTED'); }); }); + +test('SheetController - test DataFrame resizing', async () => { + const sc = new SheetController(); + GetCellsDBSetSheet(sc.sheet); + + const cell_0_0 = { + x: 0, + y: 0, + value: '', + type: 'PYTHON', + python_code: `import pandas as pd +pd.DataFrame({'A': [1, 2, 3, 4, 5], 'B': [1, 2, 3, 4, 5]})`, + last_modified: '2023-01-19T19:12:21.745Z', + } as Cell; + + await updateCellAndDCells({ starting_cells: [cell_0_0], sheetController: sc, pyodide }); + + const after_code_run_cells = sc.sheet.grid.getNakedCells(0, 0, 1, 4); + // expect(after_code_run_cells.length).toBe(3); + after_code_run_cells.forEach((cell, index) => { + console.log(cell); + // expect(cell.value).toEqual(((cell.y + 1) * 2).toString()); + if (index === 0) return; + // expect(cell.type).toEqual('COMPUTED'); + }); +}); From ef776bad5292dea17357e12cfa75f675dfc47ad9 Mon Sep 17 00:00:00 2001 From: David Kircos Date: Wed, 22 Feb 2023 15:29:27 -0700 Subject: [PATCH 2/3] replicate issue with failing unit test --- .../controller/tests/cell_evaluation.test.ts | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/src/grid/controller/tests/cell_evaluation.test.ts b/src/grid/controller/tests/cell_evaluation.test.ts index 4b9d620b1e..0c9c8ed12d 100644 --- a/src/grid/controller/tests/cell_evaluation.test.ts +++ b/src/grid/controller/tests/cell_evaluation.test.ts @@ -265,21 +265,48 @@ test('SheetController - test DataFrame resizing', async () => { const cell_0_0 = { x: 0, y: 0, - value: '', - type: 'PYTHON', - python_code: `import pandas as pd -pd.DataFrame({'A': [1, 2, 3, 4, 5], 'B': [1, 2, 3, 4, 5]})`, + value: '10', + type: 'TEXT', last_modified: '2023-01-19T19:12:21.745Z', } as Cell; await updateCellAndDCells({ starting_cells: [cell_0_0], sheetController: sc, pyodide }); - const after_code_run_cells = sc.sheet.grid.getNakedCells(0, 0, 1, 4); - // expect(after_code_run_cells.length).toBe(3); + const cell_0_1 = { + x: 0, + y: 1, + value: '10', + type: 'PYTHON', + python_code: `result = [] +repeat = int(c(0,0)) +for x in range(0, repeat): + result.append(x + repeat)`, + last_modified: '2023-01-19T19:12:21.745Z', + } as Cell; + + await updateCellAndDCells({ starting_cells: [cell_0_1], sheetController: sc, pyodide }); + + // Now resize the dataframe + const cell_0_0_update = { + x: 0, + y: 0, + value: '5', + type: 'TEXT', + last_modified: '2023-01-19T19:12:21.745Z', + } as Cell; + + await updateCellAndDCells({ starting_cells: [cell_0_0_update], sheetController: sc, pyodide }); + + // Validate the dataframe is resized + + // TODO: validate the code cell is set properly + + const after_code_run_cells = sc.sheet.grid.getNakedCells(0, 0, 0, 10); + expect(after_code_run_cells.length).toBe(5); after_code_run_cells.forEach((cell, index) => { console.log(cell); - // expect(cell.value).toEqual(((cell.y + 1) * 2).toString()); + expect(cell.value).toEqual((cell.y + 5).toString()); if (index === 0) return; - // expect(cell.type).toEqual('COMPUTED'); + expect(cell.type).toEqual('COMPUTED'); }); }); From 52325e670e72d8c318b5c1c4f612dd9ed0292d85 Mon Sep 17 00:00:00 2001 From: David Kircos Date: Wed, 22 Feb 2023 20:31:56 -0700 Subject: [PATCH 3/3] fix bug with dataframe resizing from external cell, and bug for updating deleted array cell dependents. add tests --- src/grid/actions/updateCellAndDCells.ts | 38 ++++---- .../controller/tests/cell_evaluation.test.ts | 86 +++++++++++++++++-- 2 files changed, 100 insertions(+), 24 deletions(-) diff --git a/src/grid/actions/updateCellAndDCells.ts b/src/grid/actions/updateCellAndDCells.ts index f7152f64c9..ff2cd2ec94 100644 --- a/src/grid/actions/updateCellAndDCells.ts +++ b/src/grid/actions/updateCellAndDCells.ts @@ -31,13 +31,11 @@ export const updateCellAndDCells = async (args: ArgsType) => { let seen = Array(); for (let i = 0; i < cells_to_update.length; null) { let string_id = cells_to_update[i].join(','); - if (seen.includes(string_id)) { cells_to_update.splice(i, 1); } else { i++; } - seen.push(string_id); } @@ -49,17 +47,18 @@ export const updateCellAndDCells = async (args: ArgsType) => { let cell = sheetController.sheet.getCellCopy(ref_current_cell[0], ref_current_cell[1]); let old_array_cells: Coordinate[] = []; + // keep track of previous array cells for this cell + old_array_cells = + cell?.array_cells?.map((cell) => { + return { x: cell[0], y: cell[1] }; + }) || []; + old_array_cells.unshift(); // remove this cell + // ref_current_cell is in starting_cells if (starting_cells.some((c) => c.x === ref_current_cell[0] && c.y === ref_current_cell[1])) { // if the ref_cell_to_update is the starting_cell // then we need to update the cell with data from the starting_cell - if (cell !== undefined) { - old_array_cells = - cell.array_cells?.map((cell) => { - return { x: cell[0], y: cell[1] }; - }) || []; - old_array_cells.unshift(); // remove this cell - } + const passed_in_cell = starting_cells.find((c) => c.x === ref_current_cell[0] && c.y === ref_current_cell[1]); if (passed_in_cell === undefined) continue; cell = { ...passed_in_cell }; @@ -158,14 +157,6 @@ export const updateCellAndDCells = async (args: ArgsType) => { cell.value = would_override_og_cell?.value || ''; array_cells_to_output.unshift(cell); - // if any updated cells have other cells depending on them, add to list to update - for (const array_cell of array_cells_to_output) { - // add array cells to list to update - let deps = sheetController.sheet.cell_dependency.getDependencies([array_cell.x, array_cell.y]); - - if (deps) cells_to_update.push(...deps); - } - // keep track of array cells updated by this cell cell.array_cells = array_cells_to_output.map((a_cell) => [a_cell.x, a_cell.y]); @@ -223,9 +214,20 @@ export const updateCellAndDCells = async (args: ArgsType) => { }); }); + // if any updated cells have other cells depending on them, add to list to update + for (const array_cell of array_cells_to_output) { + let deps = sheetController.sheet.cell_dependency.getDependencies([array_cell.x, array_cell.y]); + if (deps) cells_to_update.push(...deps); + } + + // any deleted cells have other cells depending on them, add to list to update + for (const array_cell of array_cells_to_delete) { + let deps = sheetController.sheet.cell_dependency.getDependencies([array_cell.x, array_cell.y]); + if (deps) cells_to_update.push(...deps); + } + // if this cell updates other cells add them to the list to update let deps = sheetController.sheet.cell_dependency.getDependencies([cell.x, cell.y]); - if (deps) cells_to_update.push(...deps); } diff --git a/src/grid/controller/tests/cell_evaluation.test.ts b/src/grid/controller/tests/cell_evaluation.test.ts index 0c9c8ed12d..23400928da 100644 --- a/src/grid/controller/tests/cell_evaluation.test.ts +++ b/src/grid/controller/tests/cell_evaluation.test.ts @@ -286,6 +286,23 @@ for x in range(0, repeat): await updateCellAndDCells({ starting_cells: [cell_0_1], sheetController: sc, pyodide }); + // Validate the dataframe is sized + const code_cell_first_run = sc.sheet.grid.getNakedCells(0, 1, 0, 1)[0]; + expect(code_cell_first_run?.value).toBe('10'); + expect(code_cell_first_run?.array_cells?.length).toBe(10); + expect(code_cell_first_run?.array_cells).toEqual([ + [0, 1], + [0, 2], + [0, 3], + [0, 4], + [0, 5], + [0, 6], + [0, 7], + [0, 8], + [0, 9], + [0, 10], + ]); + // Now resize the dataframe const cell_0_0_update = { x: 0, @@ -298,15 +315,72 @@ for x in range(0, repeat): await updateCellAndDCells({ starting_cells: [cell_0_0_update], sheetController: sc, pyodide }); // Validate the dataframe is resized - - // TODO: validate the code cell is set properly - - const after_code_run_cells = sc.sheet.grid.getNakedCells(0, 0, 0, 10); + const code_cell_after_run = sc.sheet.grid.getNakedCells(0, 1, 0, 1)[0]; + expect(code_cell_after_run?.value).toBe('5'); + expect(code_cell_after_run?.array_cells?.length).toBe(5); + expect(code_cell_after_run?.array_cells).toEqual([ + [0, 1], + [0, 2], + [0, 3], + [0, 4], + [0, 5], + ]); + + // validate the dataframe is resized and old data is cleared + const after_code_run_cells = sc.sheet.grid.getNakedCells(0, 1, 0, 10); expect(after_code_run_cells.length).toBe(5); after_code_run_cells.forEach((cell, index) => { - console.log(cell); - expect(cell.value).toEqual((cell.y + 5).toString()); + expect(cell.value).toEqual((cell.y - 1 + 5).toString()); if (index === 0) return; expect(cell.type).toEqual('COMPUTED'); }); }); + +test('SheetController - test deleted array cells update dependent cells', async () => { + const sc = new SheetController(); + GetCellsDBSetSheet(sc.sheet); + + const cell_1_2_dependent = { + x: 1, + y: 2, + value: '10', + type: 'PYTHON', + python_code: `c(0,2) + 100`, + } as Cell; + + await updateCellAndDCells({ starting_cells: [cell_1_2_dependent], sheetController: sc, pyodide }); + + const cell_0_0 = { + x: 0, + y: 0, + value: '', + type: 'PYTHON', + python_code: `[1,2,3,4,5,6,7,8,9,10]`, + } as Cell; + + await updateCellAndDCells({ starting_cells: [cell_0_0], sheetController: sc, pyodide }); + + // validate that the dependent cell is updated + const after_code_run_cell = sc.sheet.grid.getNakedCells(1, 2, 1, 2); + expect(after_code_run_cell.length).toBe(1); + expect(after_code_run_cell[0]?.value).toBe('103'); + + // Now shorten the array output + const cell_0_0_update = { + x: 0, + y: 0, + value: '', + type: 'PYTHON', + python_code: `[1,2]`, + } as Cell; + + await updateCellAndDCells({ starting_cells: [cell_0_0_update], sheetController: sc, pyodide }); + + // validate that the dependent cell is updated + const after_code_run_cell_update = sc.sheet.grid.getNakedCells(1, 2, 1, 2); + expect(after_code_run_cell_update.length).toBe(1); + expect(after_code_run_cell_update[0]?.evaluation_result?.success).toBe(false); + expect(after_code_run_cell_update[0]?.evaluation_result?.std_err).toBe( + "TypeError on line 1: unsupported operand type(s) for +: 'NoneType' and 'int'" + ); +});