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

TypeError : abs(): Argument #1 ($num) must be of type int|float, string given #3128

Closed
2 of 8 tasks
ma-gnesium opened this issue Oct 19, 2022 · 3 comments · Fixed by #3152
Closed
2 of 8 tasks

TypeError : abs(): Argument #1 ($num) must be of type int|float, string given #3128

ma-gnesium opened this issue Oct 19, 2022 · 3 comments · Fixed by #3152

Comments

@ma-gnesium
Copy link

ma-gnesium commented Oct 19, 2022

This is:

- [x] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

Formatting a number with no run-time error

What is the current behavior?

TypeError : abs(): Argument #1 ($num) must be of type int|float, string given
in PhpSpreadsheet/Style/NumberFormat/NumberFormatter.php:74

What are the steps to reproduce?

Please provide a [Minimal, Complete, and Verifiable example]

As a unit test :

<?php

use PhpOffice\PhpSpreadsheet\Style\NumberFormat;
use PHPUnit\Framework\TestCase;

class NumberFormatTest extends TestCase
{
    public function testSmallValueWithComplexFormat(): void
    {
        $result = NumberFormat::toFormattedString(1E-17, '0 000.0');

        self::assertEquals('0 000.0', $result);
    }
}

What features do you think are causing the issue

  • Reader
  • Writer
  • Styles
  • Data Validations
  • Formula Calculations
  • Charts
  • AutoFilter
  • Form Elements

Does an issue affect all spreadsheet file formats? If not, which formats are affected?

This issue is independent of the spreadsheet file format used

Which versions of PhpSpreadsheet and PHP are affected?

Don't know exactly.
I'm currently using PHPSpreadsheet version 1.25.2 and PHP 8.1.9

@ma-gnesium
Copy link
Author

I'm reconsidering this bug report.
The original intention was to format numbers using a thousands separator.

However, I've found that the correct syntax for this is to use the comma (,) character as a thousands separator, whatever the locale you're dealing with.

So the fix is simply to use the correct syntax: the TypeError is then not encountered and the result is correct.
This test passes:

    public function testSmallValueWithComplexFormat(): void
    {
        $result = NumberFormat::toFormattedString(1E-17, '0,000.0');

        self::assertEquals('0 000.0', $result);
    }

I will not be digging this issue further, but I'm wondering if we should we keep this issue open.
Indeed, if the format 0 000.0 is allowed, I wonder what the outcome of using it should be, while it certainly shouldn't produce a run-time TypeError with specific values.

@MarkBaker
Copy link
Member

MarkBaker commented Oct 27, 2022

As a mask, '0 000.0' is valid, and should be treated as a complexNumberFormatMask; it's saying that there should always be a space character in that position if there are more than 3 non-decimal digits, regardless of the locale, and if there are less than 3 non-decimal digits, then the value should be padded with 0 characters in those positions.

So 123.45 is displayed as 0 123.45; 12345.6 is displayed as 12 345.6, 12 is displayed as 0 012.0

@ma-gnesium
Copy link
Author

Thanks.

I would like to clarify the report:

Expected behavior:
Using the '0 000.0' mask, the expected display of 1e-17 should be 0 000.00000000000000001, and 1e17 should be displayed as 100000000000000 000.0.

Current behavior:
As soon as the conversion of a value to a string involves scientific notation, using a complexNumberFormatMask produces a TypeError.

Correct?

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Nov 1, 2022
Fix PHPOffice#3128 (author found a workaround but the problem remains). For some complex masks, when a cast of the cell value from float to string results in the use of scientific notation, the result of the formatting is unusable. I believe this PR solves the problem for numbers close to zero (Php cast from float to string uses scientific notation starting with 1E-5), and for a range of large numbers which are not handled correctly now.

However, I have not found a way to ensure that the results match Excel for very large numbers (1E18 or larger); this change at least ensures that the resulting string is an accurate rendition of the number (which is not the case now) even if doesn't match Excel. As an example, if you use the mask reported in the original issue `0 000.0` and enter a value of 1E90 into the cell, Excel will show it as 1 followed by 87 zeros, a space, 3 more zeros, decimal point and zero. I have not figured out how to get PhpSpreadsheet to do that; for now, it will just return the formatted value as 1 followed by 90 zeroes instead (I might have chosen to go with scientific notation instead). I will continue to think about those, but do not feel it is worth delaying the improvements in this ticket while I do so.

The affected section of code also truncated to the appropriate precision. It now rounds, as Excel does.

This seemed to be an area of code where problems might arise on 32-bit systems, and, indeed, I found something in the formatting code which had to be changed for 32-bit to work correctly. As long as I was doing that anyhow, I ran the full test suite, and found that Php8.1 had introduced some new stringencies which caused problems in a handful of places. All were found in Xls Reader, and all are corrected now.
oleibman added a commit that referenced this issue Nov 7, 2022
* Problems Formatting Very Small and Very Large Numbers

Fix #3128 (author found a workaround but the problem remains). For some complex masks, when a cast of the cell value from float to string results in the use of scientific notation, the result of the formatting is unusable. I believe this PR solves the problem for numbers close to zero (Php cast from float to string uses scientific notation starting with 1E-5), and for a range of large numbers which are not handled correctly now.

However, I have not found a way to ensure that the results match Excel for very large numbers (1E18 or larger); this change at least ensures that the resulting string is an accurate rendition of the number (which is not the case now) even if doesn't match Excel. As an example, if you use the mask reported in the original issue `0 000.0` and enter a value of 1E90 into the cell, Excel will show it as 1 followed by 87 zeros, a space, 3 more zeros, decimal point and zero. I have not figured out how to get PhpSpreadsheet to do that; for now, it will just return the formatted value as 1 followed by 90 zeroes instead (I might have chosen to go with scientific notation instead). I will continue to think about those, but do not feel it is worth delaying the improvements in this ticket while I do so.

The affected section of code also truncated to the appropriate precision. It now rounds, as Excel does.

This seemed to be an area of code where problems might arise on 32-bit systems, and, indeed, I found something in the formatting code which had to be changed for 32-bit to work correctly. As long as I was doing that anyhow, I ran the full test suite, and found that Php8.1 had introduced some new stringencies which caused problems in a handful of places. All were found in Xls Reader, and all are corrected now.

* Remove Dead Assignment

Scrutinizer will be happy now.
MarkBaker added a commit that referenced this issue Dec 21, 2022
### Added

- Extended flag options for the Reader `load()` and Writer `save()` methods
- Apply Row/Column limits (1048576 and XFD) in ReferenceHelper [PR #3213](#3213)
- Allow the creation of In-Memory Drawings from a string of binary image data, or from a stream. [PR #3157](#3157)
- Xlsx Reader support for Pivot Tables [PR #2829](#2829)
- Permit Date/Time Entered on Spreadsheet to be calculated as Float [Issue #1416](#1416) [PR #3121](#3121)

### Changed

- Nothing

### Deprecated

- Direct update of Calculation::suppressFormulaErrors is replaced with setter.
- Font public static variable defaultColumnWidths replaced with constant DEFAULT_COLUMN_WIDTHS.
- ExcelError public static variable errorCodes replaced with constant ERROR_CODES.
- NumberFormat constant FORMAT_DATE_YYYYMMDD2 replaced with existing identical FORMAT_DATE_YYYYMMDD.

### Removed

- Nothing

### Fixed

- Fixed handling for `_xlws` prefixed functions from Office365 [Issue #3245](#3245) [PR #3247](#3247)
- Conditionals formatting rules applied to rows/columns are removed [Issue #3184](#3184) [PR #3213](#3213)
- Treat strings containing currency or accounting values as floats in Calculation Engine operations [Issue #3165](#3165) [PR #3189](#3189)
- Treat strings containing percentage values as floats in Calculation Engine operations [Issue #3155](#3155) [PR #3156](#3156) and [PR #3164](#3164)
- Xlsx Reader Accept Palette of Fewer than 64 Colors [Issue #3093](#3093) [PR #3096](#3096)
- Use Locale-Independent Float Conversion for Xlsx Writer Custom Property [Issue #3095](#3095) [PR #3099](#3099)
- Allow setting AutoFilter range on a single cell or row [Issue #3102](#3102) [PR #3111](#3111)
- Xlsx Reader External Data Validations Flag Missing [Issue #2677](#2677) [PR #3078](#3078)
- Reduces extra memory usage on `__destruct()` calls [PR #3092](#3092)
- Additional properties for Trendlines [Issue #3011](#3011) [PR #3028](#3028)
- Calculation suppressFormulaErrors fix [Issue #1531](#1531) [PR #3092](#3092)
- Permit Date/Time Entered on Spreadsheet to be Calculated as Float [Issue #1416](#1416) [PR #3121](#3121)
- Incorrect Handling of Data Validation Formula Containing Ampersand [Issue #3145](#3145) [PR #3146](#3146)
- Xlsx Namespace Handling of Drawings, RowAndColumnAttributes, MergeCells [Issue #3138](#3138) [PR #3136](#3137)
- Generation3 Copy With Image in Footer [Issue #3126](#3126) [PR #3140](#3140)
- MATCH Function Problems with Int/Float Compare and Wildcards [Issue #3141](#3141) [PR #3142](#3142)
- Fix ODS Read Filter on number-columns-repeated cell [Issue #3148](#3148) [PR #3149](#3149)
- Problems Formatting Very Small and Very Large Numbers [Issue #3128](#3128) [PR #3152](#3152)
- XlsxWrite preserve line styles for y-axis, not just x-axis [PR #3163](#3163)
- Xlsx Namespace Handling of Drawings, RowAndColumnAttributes, MergeCells [Issue #3138](#3138) [PR #3137](#3137)
- More Detail for Cyclic Error Messages [Issue #3169](#3169) [PR #3170](#3170)
- Improved Documentation for Deprecations - many PRs [Issue #3162](#3162)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants