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

fix: support null in array_output file schema #491

Merged
merged 12 commits into from
May 31, 2023
26 changes: 13 additions & 13 deletions src/grid/actions/updateCellAndDCells.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { Cell } from '../../schemas';
import { Cell, ArrayOutputBase } from '../../schemas';
import { PixiApp } from '../../gridGL/pixiApp/PixiApp';
davidkircos marked this conversation as resolved.
Show resolved Hide resolved
import { Coordinate } from '../../gridGL/types/size';
import { SheetController } from '../controller/sheetController';
import { runCellComputation } from '../computations/runCellComputation';
import { ArrayOutput } from '../computations/types';
import { StringId, getKey } from '../../helpers/getKey';

interface ArgsType {
Expand Down Expand Up @@ -123,13 +122,13 @@ export const updateCellAndDCells = async (args: ArgsType) => {
let y_offset = 0;
for (const row of result.array_output) {
let x_offset = 0;
for (const cell of row as ArrayOutput) {
if (cell !== undefined)
for (const value of row as ArrayOutputBase) {
if (value !== undefined && value !== null)
array_cells_to_output.push({
x: current_cell_x + x_offset,
y: current_cell_y + y_offset,
type: 'COMPUTED',
value: cell.toString(),
value: value.toString(),
last_modified: new Date().toISOString(),
});
x_offset++;
Expand All @@ -139,14 +138,15 @@ export const updateCellAndDCells = async (args: ArgsType) => {
} else {
// 1d array
let y_offset = 0;
for (const cell of result.array_output) {
array_cells_to_output.push({
x: current_cell_x,
y: current_cell_y + y_offset,
type: 'COMPUTED',
value: cell.toString(),
last_modified: new Date().toISOString(),
});
for (const value of result.array_output as ArrayOutputBase) {
if (value !== undefined && value !== null)
array_cells_to_output.push({
x: current_cell_x,
y: current_cell_y + y_offset,
type: 'COMPUTED',
value: value.toString(),
last_modified: new Date().toISOString(),
});
y_offset++;
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/grid/computations/python/runPython.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { ArrayOutput } from '../../../schemas';

export interface runPythonReturnType {
cells_accessed: [number, number][];
success: boolean;
input_python_stack_trace: string;
input_python_std_out: string;
output_value: string | null;
array_output: (string | number | boolean)[][];
davidkircos marked this conversation as resolved.
Show resolved Hide resolved
array_output: ArrayOutput;
formatted_code: string;
}

Expand Down
4 changes: 2 additions & 2 deletions src/grid/computations/python/run_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ def __getitem__(item):
] + output_value.values.tolist()

else:
# Just return PD values
array_output = output_value.values.tolist()
# convert nan to None, return PD values list
array_output = output_value.where(output_value.notnull(), None).values.tolist()

# Convert Pandas.Series to array_output
if isinstance(output_value, pd.Series):
Expand Down
6 changes: 2 additions & 4 deletions src/grid/computations/types.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import z from 'zod';

const ArrayOutputSchema = z.array(z.union([z.string(), z.number(), z.boolean()]));
jimniels marked this conversation as resolved.
Show resolved Hide resolved
import { ArrayOutputSchema } from '../../schemas';

export const CellEvaluationResultSchema = z.object({
success: z.boolean(),
std_out: z.string().optional(),
std_err: z.string().optional(),
output_value: z.string().or(z.null()).or(z.undefined()),
cells_accessed: z.tuple([z.number(), z.number()]).array(),
array_output: z.union([ArrayOutputSchema, z.array(ArrayOutputSchema)]).optional(), // 1 or 2d array
array_output: ArrayOutputSchema,
formatted_code: z.string(),
error_span: z.tuple([z.number(), z.number()]).or(z.null()),
});

export type ArrayOutput = z.infer<typeof ArrayOutputSchema>;
export type CellEvaluationResult = z.infer<typeof CellEvaluationResultSchema>;
62 changes: 62 additions & 0 deletions src/grid/controller/tests/cell_evaluation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,3 +427,65 @@ test('SheetController - test formula dependencies', async () => {
after_code_run_cells = sc.sheet.grid.getNakedCells(0, 0, 0, 0);
expect(after_code_run_cells[0]?.value).toBe('30');
});

test('SheetController - test empty cell to be `null` in `array_output`', async () => {
const sc = new SheetController();
GetCellsDBSetSheet(sc.sheet);

// Ensure that blank cells are `null`, e.g. (2,0) should be `null`
// even when programtically getting cells
//
// [ 0 ][ 1 ][ 2 ][ 3 ]
// [0][foo][bar][ ][baz]
//
// https://github.com/quadratichq/quadratic/issues/472

const cell_0_0 = {
x: 0,
y: 0,
value: 'foo',
type: 'TEXT',
last_modified: '2023-01-19T19:12:21.745Z',
} as Cell;

const cell_1_0 = {
x: 1,
y: 0,
value: 'bar',
type: 'TEXT',
last_modified: '2023-01-19T19:12:21.745Z',
} as Cell;

const cell_3_0 = {
x: 3,
y: 0,
value: 'baz',
type: 'TEXT',
last_modified: '2023-01-19T19:12:21.745Z',
} as Cell;

const cell_0_1 = {
x: 0,
y: 1,
value: '',
type: 'PYTHON',
python_code: 'val=cells((0,0), (3,0))\nval',
last_modified: '2023-01-19T19:12:21.745Z',
} as Cell;

await updateCellAndDCells({
starting_cells: [cell_0_0, cell_1_0, cell_3_0, cell_0_1],
sheetController: sc,
pyodide,
});

const result = sc.sheet.grid.getNakedCells(0, 1, 3, 1);
expect(result[0]?.value).toBe('foo');
// If you stringify this, it will actually be `['foo','bar',null,'baz']` but
// jest converts null to undefined so we test for that
expect(result[0]?.evaluation_result?.array_output).toStrictEqual([['foo', 'bar', undefined, 'baz']]);
expect(result[1]?.value).toBe('bar');
expect(result[1]?.type).toBe('COMPUTED');
expect(result[2]?.value).toBe('baz');
expect(result[2]?.type).toBe('COMPUTED');
});
5 changes: 3 additions & 2 deletions src/schemas/GridFileV1_2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import z from 'zod';
import { GridFileV1_1 } from './GridFileV1_1';

// Shared schemas
const ArrayOutputSchema = z.array(z.union([z.string(), z.number(), z.boolean()]));
const ArrayOutputBaseSchema = z.array(z.union([z.string(), z.number(), z.boolean(), z.null()]));
const BorderDirectionSchema = z.object({
color: z.string().optional(),
type: z.enum(['line1', 'line2', 'line3', 'dotted', 'dashed', 'double']).optional(),
Expand Down Expand Up @@ -37,7 +37,7 @@ export const GridFileSchemaV1_2 = z.object({
std_err: z.string().optional(),
output_value: z.string().or(z.null()).or(z.undefined()),
cells_accessed: z.tuple([z.number(), z.number()]).array(),
array_output: z.union([ArrayOutputSchema, z.array(ArrayOutputSchema)]).optional(), // 1 or 2d array
array_output: z.union([ArrayOutputBaseSchema, z.array(ArrayOutputBaseSchema)]).optional(), // 1 or 2d array
formatted_code: z.string(),
error_span: z.tuple([z.number(), z.number()]).or(z.null()),
})
Expand Down Expand Up @@ -96,6 +96,7 @@ export const GridFileSchemaV1_2 = z.object({
version: z.literal('1.2'),
});
export type GridFileV1_2 = z.infer<typeof GridFileSchemaV1_2>;
export type ArrayOutputBase = z.infer<typeof ArrayOutputBaseSchema>;

/**
* Given a v1_1 file, update it to a v1_2 file
Expand Down
5 changes: 4 additions & 1 deletion src/schemas/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import z from 'zod';
import { GridFileV1 } from './GridFileV1';
import { GridFileV1_1, GridFileSchemaV1_1 } from './GridFileV1_1';
import { GridFileSchemaV1_2, GridFileV1_2 } from './GridFileV1_2';
import { GridFileSchemaV1_2, GridFileV1_2, ArrayOutputBase } from './GridFileV1_2';

/**
* Export types for the grid files
Expand Down Expand Up @@ -32,6 +32,9 @@ const GridFileDataSchema = GridFileSchema.pick({
});
export type GridFileData = z.infer<typeof GridFileDataSchema>;

export const ArrayOutputSchema = GridFileSchema.shape.cells.element.shape.evaluation_result.unwrap().shape.array_output;
export type ArrayOutput = z.infer<typeof ArrayOutputSchema>;
export type { ArrayOutputBase };
export type Cell = GridFile['cells'][0];
export type CellType = GridFile['cells'][0]['type'];
export type CellFormat = GridFile['formats'][0];
Expand Down
Loading