Percent/Currency value types are incorrect

[MartinDawson]

Actual:

1% & 1$ results in a valueType of NUMBER and a valueDetailedType of NUMBER_RAW.

Expected:

1% & 1$ should result in a valueType of NUMBER_PERCENT and NUMBER_CURRENCY.

Because the format is wrong if you then try and do getCellValue on it it will produce a NaN value. This means I cannot use getAllSheetsSerializedValues() and use that to build another sheet because it produces NaN results.

Steps to reproduce

Simple follow this steps here: https://handsontable.github.io/hyperformula/api/classes/hyperformula.html#getcellvaluedetailedtype

• HyperFormula version: 0.6.0
• Browser Name and version: Chrome latest
• Operating System: Windows 10

Here's my testing showing the bad results:

[izulin]

I am unable to reproduce this behaviour. Following code passes all tests for me:

    const engine = HyperFormula.buildFromArray([['1$', '1%']])
    const a1 = {sheet: 0, col: 0, row: 0}
    expect(engine.getCellValueType(a1)).toEqual('NUMBER')
    expect(engine.getCellValueDetailedType(a1)).toEqual('NUMBER_CURRENCY')
    expect(engine.getCellValueFormat(a1)).toEqual('$')
    expect(engine.getCellValue(a1)).toEqual(1)
    const b1 = {sheet: 0, col: 1, row: 0}
    expect(engine.getCellValueType(b1)).toEqual('NUMBER')
    expect(engine.getCellValueDetailedType(b1)).toEqual('NUMBER_PERCENT')
    expect(engine.getCellValueFormat(b1)).toEqual(undefined)
    expect(engine.getCellValue(b1)).toEqual(0.01)

[MartinDawson]

@izulin thanks for replying/investigating so fast. That's strange, I'll look into it and see what's happening and get back here soon

[izulin]

@MartinDawson two random things to check:

• is the '$' and '%' sign actual dollar and percent (there might be some unicode weirdness of other characters looking similarly)

• what are the engine options? you can get them with .getConfig() method

[MartinDawson]

I’ll check when I’m back home later exact my but the config I was using was just the default, ie no config so should be fine I’ll check Unicode chars are correct too, good idea Thanks

…

On Sun, 6 Jun 2021 at 11:30, Przemysław Uznański ***@***.***> wrote: @MartinDawson <https://github.com/MartinDawson> two random things to check: - is the '$' and '%' sign actual dollar and percent (there might be some unicode weirdness of other characters looking similarly) - what are the engine options? you can get them with .getConfig() method — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#686 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADSVRWZPDFMX3TVPIZ7A7RTTRNE4XANCNFSM46EKGIYQ> .

[izulin]

Trying to run the code from those unit tests might also help us debug it:

hyperformula/test/CellContentParser.spec.ts

Line 136 in 43071aa

it('currency parsing', () => {

hyperformula/test/CellContentParser.spec.ts

Line 154 in 43071aa

it('percentage parsing', () => {

[MartinDawson]

Thanks for the link, I was actually looking for that :) I’ll run those and see what I did wrong

…

On Mon, 7 Jun 2021 at 20:19, Przemysław Uznański ***@***.***> wrote: Trying to run the code from those unit tests might also help us debug it: https://github.com/handsontable/hyperformula/blob/43071aa2a822ddcea04a7b1016fa3e8dda78e751/test/CellContentParser.spec.ts#L136 https://github.com/handsontable/hyperformula/blob/43071aa2a822ddcea04a7b1016fa3e8dda78e751/test/CellContentParser.spec.ts#L154 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#686 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADSVRW3VKN35CUFCZQUONHTTRULS3ANCNFSM46EKGIYQ> .

[MartinDawson]

@izulin I tried running the unit tests and it worked fine for me on 14.x

However I tried in code sandbox and it doesn't work.

I think the issue is failing in js browser environments and working in nodejs.

See here please: https://codesandbox.io/s/quizzical-cache-6gbj8?file=/src/index.js

And observe the console.

[MartinDawson]

Here's my config:

Config {accentSensitive: false, caseSensitive: false, caseFirst: "lower", ignorePunctuation: false, chooseAddressMappingPolicy: AlwaysDense…}
accentSensitive: false
caseSensitive: false
caseFirst: "lower"
ignorePunctuation: false
chooseAddressMappingPolicy: AlwaysDense
dateFormats: Array(2)
timeFormats: Array(2)
functionArgSeparator: ","
decimalSeparator: "."
language: "enGB"
licenseKey: ""
thousandSeparator: ""
localeLang: "en"
functionPlugins: Array(0)
gpujs: undefined
gpuMode: "gpu"
smartRounding: true
matrixDetection: true
matrixDetectionThreshold: 100
evaluateNullToZero: false
nullYear: 30
precisionRounding: 14
precisionEpsilon: 1e-13
useColumnIndex: false
useStats: false
binarySearchThreshold: 20
parseDateTime: ƒ defaultParseToDateTime() {}
stringifyDateTime: ƒ defaultStringifyDateTime() {}
stringifyDuration: ƒ defaultStringifyDuration() {}
translationPackage: TranslationPackage
errorMapping: Object
nullDate: Object
leapYear1900: false
undoLimit: 20
useRegularExpressions: false
useWildcards: true
matchWholeCell: true
maxRows: 40000
maxColumns: 18278
currencySymbol: Array(1)
<constructor>: "Config"

[izulin]

The problem lies in the code relating to matrix detection, which is being phased out anyway.
Until new release comes, please do use matrixDetection: false option in the config. (e.g. HyperFormula.buildFromArray([["1%"]], { matrixDetection: false }))

In fact, this problem is not related to specific browser setting (which I am truly grateful for :) )
and can be replicated locally by running

const x = HyperFormula.buildFromArray([['1%']], {matrixDetection: true})
expect(x.getCellValue(adr('A1'))).toEqual(0.01)  // result is NaN

on e.g. 0.6.2 release

cc: @wojciechczerniak

[izulin]

(or you can switch to develop branch if it is possible)

[MartinDawson]

@izulin Turning off matrixDetection works so I'll do that for now.

Thank you

[izulin]

Happy to help!

[izulin]

(This should be closed when we release new version.)

[wojciechczerniak]

Closing as done