diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7e8aa1da7..b9b97af83 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -136,8 +136,8 @@ pub const MAX_ADOPTED_RESULT_CELLS: u64 = 4_000_000; /// Python cells from a file: a cell whose formula is `PY("…",0|1)` (a format that kept it as a /// formula) becomes a Python cell, and a Python cell whose spilled values the file cached gets /// them back as its result, so it shows them until it runs again. Results are rebuilt for at most -/// [`MAX_ADOPTED_RESULT_CELLS`] cells in all; the rest keep only their own cached value. Returns -/// the warnings. +/// [`MAX_ADOPTED_RESULT_CELLS`] cells in all; the rest keep only their own cached value, and the +/// values they spilled stay as plain values. Returns the warnings. pub fn adopt_python(wb: &mut Workbook) -> Vec { let mut budget = MAX_ADOPTED_RESULT_CELLS; let mut skipped = 0usize; @@ -168,11 +168,9 @@ pub fn adopt_python(wb: &mut Workbook) -> Vec { for (anchor, range) in spills { let Some(left) = budget.checked_sub(range.count()) else { skipped += 1; - // The cell keeps only its own value, so its cached spill goes too: the sheet would - // otherwise show spilled values that no result holds. - if let Some(r) = sh.spill_ranges.remove(&anchor) { - sh.clear_spill(r); - } + // The cell keeps only its own value; the values it spilled stay as plain values, so + // nothing shows as a spill that no result holds, and nothing the file showed is lost. + sh.settle_spill(anchor); continue; }; budget = left; @@ -189,14 +187,15 @@ pub fn adopt_python(wb: &mut Workbook) -> Vec { return Vec::new(); } vec![format!( - "The file's Python results are larger than {MAX_ADOPTED_RESULT_CELLS} cells in all, so {skipped} Python cell(s) show only their first value until they run again." + "The file's Python results are larger than {MAX_ADOPTED_RESULT_CELLS} cells in all, so {skipped} Python cell(s) have no result until they run again; the values they spilled were kept as plain values." )] } /// A file is somebody else's work: a Python cell whose code doesn't fit in Excel's /// 8,192-character formula keeps only its cached value, and a kept result that is not a /// well-formed array of at most [`gridcraft_model::python::MAX_RESULT_CELLS`] cells is dropped -/// (the cell keeps its value). Returns the warnings. +/// (the cell keeps its value). Either way the values the cell spilled stay as plain values +/// ([`gridcraft_model::Sheet::settle_spill`]). Returns the warnings. pub fn enforce_python_limits(wb: &mut Workbook) -> Vec { let mut long = Vec::new(); for si in 0..wb.sheets.len() { @@ -227,12 +226,10 @@ pub fn enforce_python_limits(wb: &mut Workbook) -> Vec { std::sync::Arc::make_mut(py).output = None; } } - // The spill of a dropped result (or of code dropped to its value) goes with it, so the - // sheet and a save no longer show the old spilled values. + // A dropped result (or code dropped to its value) leaves the values it spilled as plain + // values, as the file showed them, with no live spill. for c in dropped { - if let Some(r) = sh.spill_ranges.remove(&c) { - sh.clear_spill(r); - } + sh.settle_spill(c); } } let Some(first) = long.first() else { return vec![] }; diff --git a/crates/engine/src/tests/python_files.rs b/crates/engine/src/tests/python_files.rs index 6fa4b499c..4317bbb6b 100644 --- a/crates/engine/src/tests/python_files.rs +++ b/crates/engine/src/tests/python_files.rs @@ -353,40 +353,95 @@ fn many_cells_naming_one_long_script_take_from_one_budget() { assert_eq!(sh.value(CellRef::parse("A20000").unwrap()), Value::Number(1.0), "the others keep their values"); } +/// Plants a cached spill over `range` (anchored at its start) with `value` at `at`, as a file +/// read leaves it. +fn cached_spill(sh: &mut gridcraft_model::Sheet, range: RangeRef, spilled: &[(CellRef, Value)]) { + sh.spill_ranges.insert(range.start, range); + for (c, v) in spilled { + sh.spill.insert(*c, v.clone()); + } +} + +/// The value stored in the cell itself (not a spilled one), with whether it holds code or a +/// formula. +fn stored(sh: &gridcraft_model::Sheet, c: CellRef) -> (Value, bool) { + sh.cell(c).map(|x| (x.value.clone(), x.py.is_some() || x.formula.is_some())).unwrap_or((Value::Empty, false)) +} + #[test] -fn a_result_cut_by_the_budget_takes_its_cached_spill_with_it() { +fn a_result_cut_by_the_budget_keeps_its_cached_spill_as_values() { // Five Python cells each with a cached 1,000 x 1,000 spill: the budget rebuilds four results; - // the fifth cell keeps only its own value, and its spilled values no longer show. + // the fifth cell keeps only its own value, and the values it spilled stay as plain values. let mut wb = Workbook::new(); + let bold = wb.styles.derive(gridcraft_model::StyleId::DEFAULT, |st| st.font.bold = true); let sh = wb.sheet_mut(0).unwrap(); for i in 0..5u32 { - let anchor = CellRef::new(0, i * 1_000); + let col = i * 1_000; + let anchor = CellRef::new(0, col); sh.set_cell(anchor, Cell { py: Some(Arc::new(PyCode::new("x"))), ..Cell::value(Value::Number(1.0)) }); - sh.spill_ranges.insert(anchor, RangeRef::new(anchor, CellRef::new(999, i * 1_000 + 999))); - sh.spill.insert(CellRef::new(1, i * 1_000), Value::Number(7.0)); - sh.spill.insert(CellRef::new(999, i * 1_000 + 999), Value::Number(8.0)); + // A cell with its own content, and a formatted empty one, inside the spill. + sh.set_cell(CellRef::new(2, col), Cell::value(Value::text("own"))); + sh.set_cell(CellRef::new(3, col), Cell::styled(Value::Empty, bold)); + let spilled = [ + (CellRef::new(1, col), Value::Number(7.0)), + (CellRef::new(2, col), Value::Number(9.0)), + (CellRef::new(3, col), Value::Number(6.0)), + (CellRef::new(999, col + 999), Value::Number(8.0)), + ]; + cached_spill(sh, RangeRef::new(anchor, CellRef::new(999, col + 999)), &spilled); } let warnings = crate::io::adopt_python(&mut wb); assert!(warnings.iter().any(|w| w.contains("Python")), "{warnings:?}"); let sh = wb.sheet(0).unwrap(); let mut cut = 0; for i in 0..5u32 { - let anchor = CellRef::new(0, i * 1_000); - let rebuilt = sh.cell(anchor).and_then(|c| c.py.as_ref()).is_some_and(|p| p.output.is_some()); - let (below, corner) = (sh.value(CellRef::new(1, i * 1_000)), sh.value(CellRef::new(999, i * 1_000 + 999))); - if rebuilt { - assert_eq!((below, corner), (Value::Number(7.0), Value::Number(8.0)), "a result within the budget keeps its spill"); - assert!(sh.spill_ranges.contains_key(&anchor)); - } else { - cut += 1; - assert_eq!((below, corner), (Value::Empty, Value::Empty), "no stale spilled values beyond the anchor"); - assert!(!sh.spill_ranges.contains_key(&anchor)); - assert_eq!(sh.value(anchor), Value::Number(1.0), "the cell keeps its own cached value"); + let col = i * 1_000; + let anchor = CellRef::new(0, col); + let py = sh.cell(anchor).and_then(|c| c.py.clone()).expect("the code is kept"); + let shown = + [sh.value(CellRef::new(1, col)), sh.value(CellRef::new(2, col)), sh.value(CellRef::new(3, col)), sh.value(CellRef::new(999, col + 999))]; + assert_eq!(shown, [Value::Number(7.0), Value::text("own"), Value::Number(6.0), Value::Number(8.0)], "every result shows the same values"); + if py.output.is_some() { + assert!(sh.spill_ranges.contains_key(&anchor), "a result within the budget keeps its spill"); + continue; } + cut += 1; + assert!(!sh.spill_ranges.contains_key(&anchor), "no live spill"); + assert!(sh.spill.range(anchor..=CellRef::new(999, col + 999)).all(|(c, _)| c.col < col || c.col > col + 999), "no spilled values left"); + assert_eq!(stored(sh, anchor), (Value::Number(1.0), true), "the anchor keeps its value and its code"); + assert_eq!(stored(sh, CellRef::new(1, col)), (Value::Number(7.0), false), "a spilled value is a plain value now"); + assert_eq!(stored(sh, CellRef::new(999, col + 999)), (Value::Number(8.0), false)); + assert_eq!(stored(sh, CellRef::new(2, col)), (Value::text("own"), false), "content is never overwritten"); + assert_eq!(sh.style_id(CellRef::new(3, col)), bold, "nor a format"); } assert_eq!(cut, 1); } +#[test] +fn code_too_long_for_a_cell_keeps_its_whole_cached_result_as_values() { + let long = PyCode::new(&"x".repeat(gridcraft_model::python::MAX_FORMULA_CHARS)); + assert!(!long.fits()); + let mut wb = Workbook::new(); + let sh = wb.sheet_mut(0).unwrap(); + // A1 spills over A1:B2; D1's own value is empty. + sh.set_cell(CellRef::new(0, 0), Cell { py: Some(Arc::new(long.clone())), ..Cell::value(Value::Number(1.0)) }); + sh.set_cell(CellRef::new(1, 1), Cell::value(Value::text("own"))); + let spilled = [(CellRef::new(0, 1), Value::Number(2.0)), (CellRef::new(1, 0), Value::Number(3.0)), (CellRef::new(1, 1), Value::Number(4.0))]; + cached_spill(sh, RangeRef::new(CellRef::new(0, 0), CellRef::new(1, 1)), &spilled); + sh.set_cell(CellRef::new(0, 3), Cell { py: Some(Arc::new(long)), ..Cell::default() }); + cached_spill(sh, RangeRef::new(CellRef::new(0, 3), CellRef::new(1, 3)), &[(CellRef::new(1, 3), Value::Number(5.0))]); + let warnings = crate::io::enforce_python_limits(&mut wb); + assert!(warnings.iter().any(|w| w.contains("A1")), "{warnings:?}"); + let sh = wb.sheet(0).unwrap(); + assert!(sh.spill_ranges.is_empty() && sh.spill.is_empty(), "no live spill: {:?}", sh.spill_ranges); + assert_eq!(stored(sh, CellRef::new(0, 0)), (Value::Number(1.0), false), "the anchor keeps only its value"); + assert_eq!(stored(sh, CellRef::new(0, 1)), (Value::Number(2.0), false)); + assert_eq!(stored(sh, CellRef::new(1, 0)), (Value::Number(3.0), false)); + assert_eq!(stored(sh, CellRef::new(1, 1)), (Value::text("own"), false), "content is never overwritten"); + assert_eq!(stored(sh, CellRef::new(0, 3)), (Value::Empty, false), "an empty anchor stays empty, without code"); + assert_eq!(stored(sh, CellRef::new(1, 3)), (Value::Number(5.0), false)); +} + #[test] fn native_json_keeps_python_cells_and_their_results() { let (mut s, long) = build(); @@ -513,7 +568,7 @@ fn py_formulas_over_the_size_limit_keep_only_their_value() { } #[test] -fn a_dropped_python_result_takes_its_spill_with_it() { +fn a_dropped_python_result_keeps_its_spill_as_values() { // A1's code is over the limit and B1's result is malformed; each had spilled down to row 3, // and C1's well-formed result keeps its spill. let mut wb = Workbook::new(); @@ -536,13 +591,14 @@ fn a_dropped_python_result_takes_its_spill_with_it() { let sh = wb.sheet(0).unwrap(); for (anchor, below) in [("A1", "A2"), ("B1", "B3")] { assert!(!sh.spill_ranges.contains_key(&CellRef::parse(anchor).unwrap()), "{anchor}'s spill range stays"); - assert_eq!(sh.value(CellRef::parse(below).unwrap()), Value::Empty, "{below} still shows {anchor}'s old spill"); + let cell = sh.cell(CellRef::parse(below).unwrap()).unwrap_or_else(|| panic!("{below} is a plain value")); + assert_eq!((&cell.value, cell.py.is_none() && cell.formula.is_none()), (&Value::Number(8.0), true), "{below} keeps {anchor}'s value"); assert_eq!(sh.value(CellRef::parse(anchor).unwrap()), Value::Number(7.0), "{anchor} keeps its value"); } assert_eq!(sh.value(CellRef::parse("C2").unwrap()), Value::Number(8.0), "a sound result keeps its spill"); - // A save writes no value for the dropped spills. + // A save writes the kept values as plain values, and the sound spill as a spill. let xml = sheet_xml(&gridcraft_xlsx::write_xlsx(&wb).unwrap()); - assert!(!xml.contains("r=\"A2\"") && !xml.contains("r=\"B3\""), "{xml}"); + assert!(xml.contains("8") && xml.contains("8"), "{xml}"); assert!(xml.contains("r=\"C2\""), "{xml}"); } diff --git a/crates/model/src/sheet.rs b/crates/model/src/sheet.rs index 433451277..102a41317 100644 --- a/crates/model/src/sheet.rs +++ b/crates/model/src/sheet.rs @@ -215,6 +215,29 @@ impl Sheet { } } + /// Turns the spill anchored at `anchor` into plain values, for a Python cell that keeps only + /// its value: the range is forgotten, and each spilled value in it becomes its cell's value, + /// unless the cell holds content of its own (its format is kept either way). The work is + /// bounded by the spilled values stored, not by the size of the range. + pub fn settle_spill(&mut self, anchor: CellRef) { + let Some(r) = self.spill_ranges.remove(&anchor) else { return }; + let inside: Vec = self.spill.range(r.start..=r.end).map(|(c, _)| *c).filter(|c| r.contains(*c)).collect(); + for c in inside { + let Some(v) = self.spill.remove(&c) else { continue }; + if v.is_empty() || c == anchor { + continue; + } + match self.cells.get_mut(c) { + Some(cell) if cell.has_content() => {} + Some(cell) => cell.value = v, + None => { + let style = self.style_id(c); + self.cells.set(c, Cell::styled(v, style)); + } + } + } + } + /// Displayed value of a cell: its own value, or a spilled value. pub fn value(&self, c: CellRef) -> Value { match self.cells.get(c) { diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 2626586cd..86c4df6bd 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -100,7 +100,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | `.xlsx` | read and write | Written as [MS-XLSX] 2.3.10 specifies: the cell's formula is `_xlfn._xlws.PY(i,n)`, where `i` is the index of the cell's script in the workbook's Python part (`xl/python.xml`, `application/vnd.ms-excel.python+xml`) and `n` the return type, 0 for an Excel value or 1 for a Python object, with the cached value. The code keeps its `xl()` references as text, so the formula has no further arguments. A spilling result is written as a dynamic array with its spilled values cached. Read back with or without the `_xlfn.` and `_xlws.` prefixes: an index names a script of the Python part, whose `%Px%` references become the text of the formula's argument `x`, and the older form with the code in string literals, `PY("…"&"…",n)`, still reads. So workbooks laid out as the format specifies open with Python cells, their cached values and spills. Python errors are cached as `#VALUE!`, as Excel caches `#SPILL!` and `#CALC!` | | `.xls`, `.xlsb`, `.ods` | read | A `PY("…",0)` or `PY("…",1)` formula read from any format becomes a Python cell; the `.xlsb` and `.ods` imports keep values only, so Python cells there come in as their cached values | | GridCraft JSON | read and write | The whole Python cell: code, return type, the last result and the exact error | -| Limits on open | — | Code that doesn't fit Excel's 8,192-character formula as `PY("…",n)` keeps only its cached value, with a warning, and so does a formula naming a script the Python part doesn't hold. The Python part is read up to 64 MB and 1,000,000 scripts, and the cells take at most 64 MB of code from it in all (several formulas can name one script). A kept result that is not a well-formed array of at most 1,000,000 cells is dropped, a spill range larger than that is ignored, and results are rebuilt for at most 4,000,000 cells over the workbook. Opening runs nothing | +| Limits on open | — | Code that doesn't fit Excel's 8,192-character formula as `PY("…",n)` keeps only its cached value, with a warning, and so does a formula naming a script the Python part doesn't hold. The Python part is read up to 64 MB and 1,000,000 scripts, and the cells take at most 64 MB of code from it in all (several formulas can name one script). A kept result that is not a well-formed array of at most 1,000,000 cells is dropped, a spill range larger than that is ignored, and results are rebuilt for at most 4,000,000 cells over the workbook. A Python cell that keeps only its value (code too long or not in the part, a damaged part, a dropped result, a result past that budget) keeps the values it spilled as plain values, without overwriting a cell's own content or format. Opening runs nothing | | Not kept | — | Python object results stored as rich values (Excel's `richData` parts) come in as their cached values. The Python part's environment definition (its id and initialization code) is not read, and a save writes none | ## Work (76–114 h)