fix(formula): sum get error when cell has error

packages/engine-formula/src/engine/reference-object/base-reference-object.ts

@@ -29,6 +29,7 @@ import {
     NumberValueObject,
     StringValueObject,
 } from '../value-object/primitive-object';
+import { getCellValue } from '../utils/cell';
 
 export type NodeValueType = BaseValueObject | BaseReferenceObject | AsyncObject | AsyncArrayObject;
 
@@ -332,7 +333,7 @@ export class BaseReferenceObject extends ObjectClassType {
     }
 
     getCellValueObject(cell: ICellData) {
-        const value = cell.v || 0;
+        const value = getCellValue(cell);
         if (ERROR_TYPE_SET.has(value as ErrorType)) {
             return new ErrorValueObject(value as ErrorType);
         }

packages/engine-formula/src/engine/utils/__tests__/cell.spec.ts

@@ -0,0 +1,46 @@
+/**
+ * Copyright 2023-present DreamNum Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { describe, expect, it } from 'vitest';
+
+import type { ICellData } from '@univerjs/core';
+import { getCellValue } from '../cell';
+
+describe('Test cell', () => {
+    it('Function getCellValue', () => {
+        const cell1: ICellData = {
+            p: {
+                id: 'p',
+                body: {
+                    dataStream: 'test\r\n',
+                },
+                documentStyle: {},
+            },
+        };
+
+        const cell2 = {
+            v: 2,
+        };
+
+        const cell3 = {
+            f: '',
+        };
+
+        expect(getCellValue(cell1)).toBe('test');
+        expect(getCellValue(cell2)).toBe(2);
+        expect(getCellValue(cell3)).toBe(0);
+    });
+});

packages/engine-formula/src/engine/utils/cell.ts

@@ -0,0 +1,40 @@
+/**
+ * Copyright 2023-present DreamNum Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { DEFAULT_EMPTY_DOCUMENT_VALUE } from '@univerjs/core';
+import type { ICellData, Nullable } from '@univerjs/core';
+
+export function getCellValue(cell: Nullable<ICellData>) {
+    if (cell === null) {
+        return 0;
+    }
+
+    if (cell?.p) {
+        const body = cell?.p.body;
+
+        if (body == null) {
+            return 0;
+        }
+
+        const data = body.dataStream;
+        const lastString = data.substring(data.length - 2, data.length);
+        const newDataStream = lastString === DEFAULT_EMPTY_DOCUMENT_VALUE ? data.substring(0, data.length - 2) : data;
+
+        return newDataStream;
+    }
+
+    return cell?.v || 0;
+}

packages/engine-formula/src/engine/value-object/__tests__/array-value-object.spec.ts

@@ -87,6 +87,54 @@ describe('arrayValueObject test', () => {
         });
     });
 
+    describe('Count', () => {
+        it('Normal count', () => {
+            const originValueObject = new ArrayValueObject({
+                calculateValueList: transformToValueObject([
+                    [1, ' ', 1.23, true, false],
+                    [0, '100', '2.34', 'test', -3],
+                ]),
+                rowCount: 2,
+                columnCount: 5,
+                unitId: '',
+                sheetId: '',
+                row: 0,
+                column: 0,
+            });
+            expect(originValueObject.count()?.getValue()).toBe(6);
+        });
+        it('CountA', () => {
+            const originValueObject = new ArrayValueObject({
+                calculateValueList: transformToValueObject([
+                    [1, ' ', 1.23, true, false],
+                    [0, '100', '2.34', 'test', -3],
+                ]),
+                rowCount: 2,
+                columnCount: 5,
+                unitId: '',
+                sheetId: '',
+                row: 0,
+                column: 0,
+            });
+            expect(originValueObject.countA()?.getValue()).toBe(10);
+        });
+        it('CountBlank', () => {
+            const originValueObject = new ArrayValueObject({
+                calculateValueList: transformToValueObject([
+                    [1, ' ', 1.23, true, false],
+                    [0, '100', '2.34', 'test', -3],
+                ]),
+                rowCount: 2,
+                columnCount: 5,
+                unitId: '',
+                sheetId: '',
+                row: 0,
+                column: 0,
+            });
+            expect(originValueObject.countBlank()?.getValue()).toBe(0);
+        });
+    });
+
     describe('pick', () => {
         it('normal', () => {
             const pickArrayValueObject = new ArrayValueObject({
@@ -143,6 +191,34 @@ describe('arrayValueObject test', () => {
         });
     });
 
+    describe('sum', () => {
+        it('normal', () => {
+            expect(originArrayValueObject.sum().getValue()).toStrictEqual(120);
+        });
+
+        // like numpy array
+        // [
+        //     [1, 0, 1.23, 1, 0],
+        //     [0, 100, 2.34, 0, -3],
+        // ]
+        it('nm multiple formats', () => {
+            const originValueObject = new ArrayValueObject({
+                calculateValueList: transformToValueObject([
+                    [1, ' ', 1.23, true, false],
+                    [0, '100', '2.34', 'test', -3],
+                ]),
+                rowCount: 2,
+                columnCount: 5,
+                unitId: '',
+                sheetId: '',
+                row: 0,
+                column: 0,
+            });
+
+            expect(originValueObject.sum().getValue()).toStrictEqual(101.57);
+        });
+    });
+
     describe('mean', () => {
         it('normal', () => {
             expect(originArrayValueObject.mean().getValue()).toStrictEqual(8);
@@ -167,7 +243,7 @@ describe('arrayValueObject test', () => {
                 column: 0,
             });
 
-            expect(originValueObject.mean().getValue()).toStrictEqual(10.257);
+            expect(originValueObject.mean().getValue()).toStrictEqual(16.928333333333335);
         });
     });
 
@@ -176,7 +252,7 @@ describe('arrayValueObject test', () => {
             expect(originArrayValueObject.var().getValue()).toStrictEqual(18.666666666666668);
         });
 
-        it('nm multiple formats', () => {
+        it('var nm multiple formats', () => {
             const originValueObject = new ArrayValueObject({
                 calculateValueList: transformToValueObject([
                     [1, ' ', 1.23, true, false],
@@ -190,7 +266,7 @@ describe('arrayValueObject test', () => {
                 column: 0,
             });
 
-            expect(originValueObject.var().getValue()).toStrictEqual(896.592801);
+            expect(originValueObject.var().getValue()).toStrictEqual(1382.9296138888888);
         });
     });
 
@@ -213,7 +289,7 @@ describe('arrayValueObject test', () => {
                 column: 0,
             });
 
-            expect(originValueObject.std().getValue()).toStrictEqual(29.94315950263098);
+            expect(originValueObject.std().getValue()).toStrictEqual(37.187761614392564);
         });
     });
 
@@ -298,7 +374,11 @@ describe('arrayValueObject test', () => {
         });
 
         it('ValueObjectFactory create StringValueObject ', () => {
-            const stringValueObject = ValueObjectFactory.create('test');
+            let stringValueObject = ValueObjectFactory.create('test');
+
+            expect(stringValueObject.isString()).toBeTruthy();
+
+            stringValueObject = ValueObjectFactory.create(' ');
 
             expect(stringValueObject.isString()).toBeTruthy();
         });

packages/engine-formula/src/engine/value-object/array-value-object.ts

@@ -729,7 +729,8 @@ export class ArrayValueObject extends BaseValueObject {
     override sum() {
         let accumulatorAll: BaseValueObject = new NumberValueObject(0);
         this.iterator((valueObject) => {
-            if (valueObject == null) {
+            // 'test', ' ',  blank cell, TRUE and FALSE are ignored
+            if (valueObject == null || valueObject.isString() || valueObject.isBoolean() || valueObject.isNull()) {
                 return true; // continue
             }
 
@@ -801,11 +802,8 @@ export class ArrayValueObject extends BaseValueObject {
     override count() {
         let accumulatorAll: BaseValueObject = new NumberValueObject(0);
         this.iterator((valueObject) => {
-            if (valueObject == null) {
-                return true; // continue
-            }
-
-            if (valueObject.isError() || valueObject.isString() || valueObject.isNull()) {
+            // 'test', ' ',  blank cell, TRUE and FALSE are ignored
+            if (valueObject == null || valueObject.isError() || valueObject.isString() || valueObject.isNull() || valueObject.isBoolean()) {
                 return true; // continue
             }
             accumulatorAll = accumulatorAll.plusBy(1) as BaseValueObject;
@@ -817,11 +815,7 @@ export class ArrayValueObject extends BaseValueObject {
     override countA() {
         let accumulatorAll: BaseValueObject = new NumberValueObject(0);
         this.iterator((valueObject) => {
-            if (valueObject == null) {
-                return true; // continue
-            }
-
-            if (valueObject.isNull()) {
+            if (valueObject == null || valueObject.isNull()) {
                 return true; // continue
             }
 
@@ -834,7 +828,7 @@ export class ArrayValueObject extends BaseValueObject {
     override countBlank() {
         let accumulatorAll: BaseValueObject = new NumberValueObject(0);
         this.iterator((valueObject) => {
-            if (valueObject != null) {
+            if (valueObject != null && !valueObject.isNull()) {
                 return true; // continue
             }
 
@@ -1066,8 +1060,8 @@ export class ArrayValueObject extends BaseValueObject {
     override mean(): BaseValueObject {
         const sum = this.sum();
 
-        // Count strings in
-        const count = this.countA();
+        // Like sum, ignore strings and booleans
+        const count = this.count();
 
         return sum.divided(count);
     }
@@ -1095,35 +1089,39 @@ export class ArrayValueObject extends BaseValueObject {
         return allValue.get(0, (count - 1) / 2);
     }
 
+    // TODO ddof, ignore strings and booleans
     override var(): BaseValueObject {
         const mean = this.mean();
 
         // let isError = null;
-        const squaredDifferences: BaseValueObject[][] = [];
-        this.iterator((valueObject: Nullable<BaseValueObject>, row: number, column: number) => {
-            if (valueObject == null || valueObject.isError() || valueObject.isString()) {
-                valueObject = new NumberValueObject(0);
+        const squaredDifferences: BaseValueObject[][] = [[]];
+        this.iterator((valueObject: Nullable<BaseValueObject>) => {
+            // for VARPA and VARA, strings and FALSE are counted as 0, TRUE is counted as 1
+            // for VAR.S/VAR, or VAR.P/VARP, strings,TRUE and FALSE are ignored
+            // Since sum ignores strings and booleans, they are ignored here too, and VAR.S and VAR.P are used more
+
+            // VAR.S assumes that its arguments are a sample of the population, like numpy.var(data, ddof=1)
+            // VAR.P assumes that its arguments are the entire population, like numpy.var(data, ddof=0)
+            // numpy.var uses ddof=0 by default, so we use ddof=0 here
+            if (valueObject == null || valueObject.isError() || valueObject.isString() || valueObject.isBoolean() || valueObject.isNull()) {
+                return;
             }
 
-            let baseValueObject = valueObject.minus(mean).pow(new NumberValueObject(2, true));
+            const baseValueObject = valueObject.minus(mean).pow(new NumberValueObject(2, true));
 
             if (baseValueObject.isError()) {
-                baseValueObject = new NumberValueObject(0);
-            }
-
-            if (squaredDifferences[row] == null) {
-                squaredDifferences[row] = [];
+                return;
             }
 
-            squaredDifferences[row][column] = baseValueObject;
+            squaredDifferences[0].push(baseValueObject);
         });
 
         const { _rowCount, _columnCount, _unitId, _sheetId, _currentRow, _currentColumn } = this;
 
         const squaredDifferencesArrayObject = new ArrayValueObject({
             calculateValueList: squaredDifferences,
-            rowCount: _rowCount,
-            columnCount: _columnCount,
+            rowCount: 1,
+            columnCount: squaredDifferences[0].length,
             unitId: _unitId,
             sheetId: _sheetId,
             row: _currentRow,
@@ -1133,6 +1131,14 @@ export class ArrayValueObject extends BaseValueObject {
         return squaredDifferencesArrayObject.mean();
     }
 
+    /**
+     * STDEV.P: ddof=0, ignore strings and booleans
+     * STDEV.S: ddof=1, ignore strings and booleans
+     *
+     * STDEVPA: ddof=0,
+     * STDEVA: ddof=1,
+     * @returns
+     */
     override std(): BaseValueObject {
         const variance = this.var();
 
@@ -1433,7 +1439,8 @@ export class ArrayValueObject extends BaseValueObject {
                 if (currentValue.isError()) {
                     result[r][column] = currentValue as ErrorValueObject;
                 } else if (valueObject.isError()) {
-                    result[r][column] = new ErrorValueObject(ErrorType.VALUE);
+                    // 1 + #NAME? gets #NAME?
+                    result[r][column] = valueObject;
                 } else {
                     switch (batchOperatorType) {
                         case BatchOperatorType.PLUS: