* Initial work enabling Excel function implementations for handling arrays as aguments when used in "array formulae".
So far:
- handling for single argument functions
- for functions where only one of the arguments is an array (a matrix or a row/column vector)
- for when there are two array arguments, and one is a row vector, the other a column vector
- for when there are either 2 row vectors, or 2 column vectors
- for a matrix and either a row or column vector
Will work ok, as long as there are no more than two array arguments; still need to identify the logic to apply when there are more than two arrays; or there are two that aren't an already supported row vector/column vector pairing (ie two matrices).
* Throw an exception if we have three or more array arguments (after flattening) passed to a supported function until we can identify the abstruse non-euclidian logic behind how Excel handles building, using and presenting those n-dimensional result arrays
* Implement array arguments for the DATE() function so that we can verify that paired arrays/vectors work with functions that support more than 2 arguments
* Implement array arguments for the many of the Math/Trig functions
* Update change log
* Implementation of the SEQUENCE() Excel365 function
Note that the Calculation Engine does not yet support the Spill operator, or spilling functions
* Handle the use-case of step = 0; and tests for exception handling for invalid arguments
* Update Change Log
* Refinement for XIRR
Fix#2469. The algorithm used for XIRR is known not to converge in some cases, some of which are because the value is legitimately unsolvable; for others, using a different guess might help.
The algorithm uses continual guesses at a rate to hopefully converge on the solution. The code in Python package xirr (https://github.com/tarioch/xirr/) suggests a refinement when this rate falls below -1. Adopting this refinement solves the problem for the data in issue 2469 without any adverse effect on the existing tests. My thanks to @tarioch for that refinement.
The data from 2469 is, of course, added to the test cases. The user also mentions that an initial guess equal to the actual result doesn't converge either. A test is also added to confirm that that case now works.
The test cases are changed to run in the context of a spreadsheet rather than by direct calls to XIRR calculation routine. This revealed some data validation errors which are also cleaned up with this PR. This suggests that other financial tests might benefit from the same change; I will look into that.
* More Unit Tests
From https://github.com/RayDeCampo/java-xirr/blob/master/src/test/java/org/decampo/xirr/XirrTest.javahttps://github.com/tarioch/xirr/blob/master/tests/test_math.py
Note that there are some cases where the PHP tests do not converge, but the non-PHP tests do. I have confirmed in each of those cases that Excel does not converge, so the PhpSpreadsheet results are good, at least for now. The discrepancies are noted in comments in the test member.
Replace mock tests with real ones when possible. The original tests are all still present; they just take place in a more representative scenario.
After this, there will be 4 remaining uses of mocking. Of these, 3 are needed for scenarios which are otherwise hard to test - WebServiceTest, CellsTest, and SampleCoverageTest. For the other one, AutoFilterTest, I just can't figure out what it's trying to accomplish, so have left it alone.
This change is almost entirely restricted to tests. There is a one-line change in src. When the first argument passed to OFFSET is null or nullstring, the returned value is currently 0. However, according to the documentation for Excel, it should be `#VALUE!`. The code is changed accordingly.
See issue #2123. HLOOKUP needs to do some conversions between column numbers and letters which it had not been doing.
HLOOKUP tests were performed using direct calls to the function in question rather than in the context of a spreadsheet. This contributed to keeping this error obscured even though there were, in theory, sufficient test cases. The tests are changed to perform in spreadsheet context. For the most part, the test cases are unchanged. One of the expected results was wrong; it has been changed, and a new case added to cover the case it was supposed to be testing.
After getting the HLOOKUP tests in order, it turned out that a test using literal arrays which had been succeeding now failed. The array constructed by the literals are considerably different than those constructed using spreadsheet cells; additional code was added to handle this situation.
My bulletproofing of these tests was not yet sufficient. Although I have never had a failure in probably thousands of tests, one user submitted a PR which did fail testing NOW, fortunately not in a test that is required to pass. The problem is that it is not sufficient merely to set the cell value inside a do-while loop; it is necessary to calculate it in order to cache its result so that results based on that cell will be internally consistent.
No source code is changed for this PR, just some tests.
Just reviewing Scrutinizer's list of "bugs". There are 19 ascribed to me. For some, I will definitely take no action (e.g. use of bitwise operators in AND, OR, and XOR functions). However, where I can clean things up so that Scrutinizer is satisfied and the resulting code is not too contorted, I will make an attempt.
This PR corrects 2 problems according to Scrutinizer, and 1 per Phpstan. Only test members are involved.
* Improve Identification of Samples in Coverage Report
The Phpunit coverage report currently contains bullet items like `PhpOffice\PhpSpreadsheetTests\Helper\SampleTest\testSample with data set "49"`. This extremely simple change takes advantage of Phpunit's ability to accept an array with keys which are either strings or integers, by using the sample filenames as the array keys rather than sequential but otherwise meaningless integers (e.g. `49` in the earlier cited item). The bullet item will now read `PhpOffice\PhpSpreadsheetTests\Helper\SampleTest\testSample with data set "Basic/38_Clone_worksheet.php"`.
* Fix for Issue 2158 (AverageIf Calculation Problem)
Issue #2158 reports an error calculating AverageIf because a function returns null rather than a string. There turn out to be several components to this problem:
- The nominal fix to the problem is to add some null-to-nullstring coercion in DatabaseAbstract.
- This fixes the error, but does not necessarily lead to the correct result because buildQuery treats values of null and null-string identically, whereas Excel does not. So change that to treat null-string as any other string.
- But that doesn't lead to the correct result either. That's because Functions/ifCondition recognizes a null string, but then continues to (over-)process it until it returns the wrong result. Fix this problem in conjunction with the other two, and we finally get the correct result.
A new unit test is added for AVERAGEIF, and new test cases are added for SUMIF. In each case, there are complementary tests for conditions of null and null-string, and the results agree with Excel. There may or may not be value in adding new tests to other functions, and I will be glad to do so for any functions which you care to identify, but no existing tests broke as a result of these changes.
* PHP8.1 Deprecation Passing Null to String Function
For each of the files in this PR, one or more statements can pass a null to string functions like strlower. This is deprecated in PHP8.1, and, when deprecated messages are enabled, causes many tests to error out. In every case, use coercion to pass null string rather than null.
* TextData - Minor Changes, Test Coverage
Per agreement on a previous push, I looked into standardizing the initialization of the TextData functions (like Engineering and MathTrig), with particular regard for avoiding multiple later null coercions. This simplifies the code quite a bit. This PR also increases coverage to 100% for all TextData modules. All entries in Phpstan baseline for non-deprecated TEXTDATA functions are removed. There were some minor bugfixes.
Whereas Excel (and Gnumeric) treat booleans when supplied as strings as 'TRUE' or 'FALSE', ODS treats them as '1' or '0'. Unlike Excel, ODS generally does not allow bool for int arguments; it does, however, allow them for FIND and SEARCH. ODS allows boolean for into for SUBSTITUTE even though Excel doesn't. ODS allows bool for string for NUMBERVALUE and VALUE even though Excel doesn't. ODS accepts 0 as an argument for CHAR; Excel doesn't. Most of this seems like random decisions on the part of the developers; I've done my best to follow the products in each case. There is a new test member devoted to ODS tests.
Gnumeric has an anomaly vis-a-vis the others - if length is supplied to LEFT/MID/RIGHT as null, Gnumeric treats it as 0 rather than 1.
All tests now take place in the context of a spreadsheet ...
Except for RETURNSTRING, which is not the implementation of an Excel function, and is referred to in the rest of PhpSpreadsheet only in the unit tests for itself. It should probably be deprecated, but that is not part of this PR, just in case there is some reason for it that I couldn't discern.
I have tried to make the first line of each doc block identify the Excel function name rather than its name in PhpSpreadsheet. I think it makes things more comprehensible.
Some tests call Settings::setLocale, but there was no Settings::getLocale. At the end of the tests which do it, they invoke setLocale('EN-US'), which, in a practical sense, is sufficient. However, in theory it would be better for them to get the current locale before changing it, then changing it back to the original when the time came. I have added getLocale and made the appropriate testing change.
The CHAR function took an interesting turn. One can set the value of a cell to, say, CHAR(2), the ASCII/UTF-8 representation of a control character, which is not legal in certain contexts. The only Reader/Writer that could handle this without problems is Xls, which deals with binary data all the time. However, if you tried to write it to Xlsx, Excel would not be able to open the resulting file because of what it considers an illegal character. I changed the Xlsx writer to escape such characters when writing the value of a string function. I did not make any other changes to the Xlsx writer - it seems to me that setting a cell to CHAR(2) is legitimate, but setting it to say `"\x02"` seems less likely to be legitimate, so the latter will still fail (although `="\x02"` should work). The Xlsx reader already supports the escape mechanism that I added to the writer.
CHAR control character and Ods - not supported by either Reader or Writer. I did not attempt to add this now. There is lots still missing from ODS, and this item just can't be a high priority amongst all of those.
CHAR control character and Csv - it is supported by reader and writer if the file has a csv extension. However, trying to guess the mime type without an extension - the control character makes mime_get_type guess application/octet-stream, and PhpSpreadsheet therefore thinks that Csv can't read it.
CHAR control character and Html. Actual use of the control character in the file is subject to the same problems as Xml (i.e. Xlsx and Ods). It wasn't terribly difficult to get the Html Writer to change `"\x02"` to "``". I believe that this is technically legal; however, DOMDocument.loadHTML rejects it as an illegal entity, and I am not convinced that it is wrong to do so, so I haven't changed the Html writer.
* Scrutinizer
Correct 3 minor errors.
* Additional unit tests for VLOOKUP() and HLOOKUP()
* Additional unit tests for CHOOSE()
* Unit tests for HYPERLINK() function
* Fix CHOOSE() test for spillage
As issue #2042 documents, SUM behaves differently with invalid strings depending on whether they come from a cell or are used as literals in the formula. SUM is not alone in this regard; COUNTA is another function within this behavior, and the solution to this one is modeled on COUNTA. New tests are added for SUM, and the resulting tests are duplicated to confirm correct behavior for both cells and literals.
Samples 16 (CSV), 17 (Html), and 21 (PDF) were adversely affected by this problem. 17 and 21 were immediately fixed, but 16 had another problem - Excel was not interpreting the UTF8 currency symbols correctly, even though the file was saved with a BOM. After some experimenting, it appears that the `sep=;` line generated by setExcelCompatibility(true) causes Excel to mis-handle the file. This seems like a bug - there is apparently no way to save a UTF-8 CSV with non-ASCII characters which specifies a non-standard separator which Excel will open correctly. I don't know if this is a recent change or if it is just the case that nobody noticed this problem till now. So, I changed Sample 16 to use setUseBom rather than setExcelCompatibility, which solved its problem. I then added new tests for setExcelCompatibility, with documentation of this problem.
Both methods used to optionally return null if passed a
second argument. This second argument was removed entirely and the
method always returns a RowDimension or ColumnDimension respectively
(possibly creating it if needed).
This make the API more predictable and easier to do static analysis
with tools such as PHPStan.
If you relied on that second parameter, you should instead use the
`Worksheet::getRowDimensions()` or `Worksheet::getColumnDimensions()` and
check for existence yourself before calling the getters.
* DateTimeExcel - Change Names of funcWhatever to evaluate
Per discussions while MathTrig was being broken up, this would help standardize the code. This PR applies that standardization to the DateTimeExcel family of functions.
The deprecation messages in DateTime.php are changed to match the style used in PR #2005.
All Phpstan grandfathered errors (about 25) in DateTimeExcel are fixed and removed from baseline. A small number (about 5) of phpstan annotations in the source members in that directory are also fixed and eliminated.
* MathTrig - Fix Phpstan Accomodations
This should be the last of my mass changes to MathTrig. All he Phpstan violations found in baseline which are part of MathTrig are now fixed and removed from baseline. There were about 20 of these.
* MathTrig - Change Names of funcWhatever to evaluate
Per discussions while MathTrig was being broken up, this would help standardize the code. That idea was adopted partway through the breakup. This PR applies that standardization to the earlier efforts. A similar effort is required for DateTime; that will come later. This PR replaces #2006.
The only 2 remaining funcWhatevers in MathTrig are both in SUM, which required two different methods depending on whether or not string parameters were to be ignored. It seems appropriate to leave those method names non-standardized in order to require a decision about which is to be used if they are invoked internally.
3 Phpstan grandfathered errors were eliminated as part of this change, and its baseline has changed accordingly.
Co-authored-by: Mark Baker <mark@lange.demon.co.uk>
* Improved Support for INDIRECT, ROW, and COLUMN Functions
This should address issues #1913 and #1993. INDIRECT had heretofore not supported an optional parameter intended to support addresses in R1C1 format which was introduced with Excel 2010. It also had not supported the use of defined names as an argument. This PR is a replacement for #1995, which is currently in draft status and which I will close in a day or two.
The ROW and COLUMN functions also should support defined names. I have added that, and test cases, with the latest push. ROWS and COLUMNS already supported it correctly, but there had been no test cases. Because ROW and COLUMN can return arrays, and PhpSpreadsheet does not support dynamic arrays, I left the existing direct-call tests unchanged to demonstrate those capabilities.
The unit tests for INDIRECT had used mocking, and were sorely lacking (tested only error conditions). They have been replaced with normal, and hopefully adequate, tests. This includes testing globally defined names, as well as locally defined names, both in and out of scope.
The test case in 1913 was too complicated for me to add as a unit test. The main impediments to it are now removed, and its complex situation will, I hope, be corrected with this fix.
INDIRECT can also support a reference of the form Sheetname!localName when localName on its own would be out of scope. That functionality is added. It is also added, in theory, for ROW and COLUMN, however such a construction is rejected by the Calculation engine before passing control to ROW or COLUMN. It might be possible to change the engine to allow this, and I may want to look into that later, but it seems much too risky, and not nearly useful enough, to attempt to address that as part of this change.
Several unusual test cases (leading equals sign, not-quite-as-expected name definition in file, complex indirection involving concatenation and a dropdown list) were suggested by @MarkBaker and are included in this request.
* More Financial function extracts, this time looking at the Periodic Cashflow functions
* Initial extract of Constant Periodic Interest and Payment functions
* Continue MathTrig Breakup - Completion!
Continuing the process of breaking MathTrip.php up into smaller classes. This round takes care of everything that was left:
- ABS
- DEGREES
- EXP
- RADIANS
- SQRT
- SQRTPI
- SUMSQ, SUMX2MY2, SUMX2PY2, SUMXMY2
The only notable logic change was that the 3 SUMX* functions had accepted arrays of unlike length; in that condition, they now return N/A, as Excel does. There had been no tests for this condition.
All the functions in MathTrig.php are now deprecated. Except for COMBIN, the test suite executes them only from MathTrig MovedFunctionsTest. COMBIN is still directly called by some Statistics Binomial functions which have not yet had the opportunity to be re-coded for the new location.
Co-authored-by: Mark Baker <mark@lange.demon.co.uk>
* Let's start with some appeasements to phpstan, just to reduce the baseline
* Appeasements to phpstan, taking the number of reported errors down to just 61
* Extract Normal and Standard Normal Distributions from the Statistical Class
* Extract ZTest from the Statistical Class, and move it to the Standard Normal Distribution class
Additional unit tests for NORMINV()
* Extract LogNormal distribution functions from Statistical
* Continue MathTrig Breakup - Penultimate?
Continuing the process of breaking MathTrip.php up into smaller classes. This round takes care of about half of what is left, so perhaps one round after this one will finish the job:
- ARABIC
- COMBIN; also implemented COMBINA
- FACTDOUBLE
- GCD (which accepts and ignores empty cells as arguments, but returns VALUE if all the arguments are that way; LCM does the same)
- LOG_BASE, LOG10, LN
- implemented MUNIT
- MOD
- POWER
- RAND, RANDBETWEEN (RANDARRAY is too complicated to implement with this ticket)
As you can see from the description, there are some functions which were combined in a single class. When not combined, I adopted PowerKiki's suggestion of using "execute" as the function name.
Co-authored-by: Mark Baker <mark@lange.demon.co.uk>
* Extract a few more Distribution functions from Statistical; this time EXPONDIST() and HYPGEOMDIST()
* Extract the F Distribution (although only F.DIST() is implemented so far
* Updae docblocks
* PHPCS
* Extract Binomial Distribution functions from Statistical
Replace the old MS algorithm for CRITBINOM() (which has now been replaced with te BINOM.INV() function) with a brute force approach - I'll look to refine it later. The MS algorithm is no longer documented, and the implementation produced erroneous results anyway
* Exract the NEGBINOMDIST() function as well; still need to add a cumulative flag to support the additional argument for the newer NEGBINOM.DIST() function
* Rationalise validation of probability arguments
* Difference in variance calculations between Excel/Gnumeric and Open/LibreOffice
* Simplify STDEV() function logic by remembering that STDEV() is simply the square root of VAR(), so we can simply use the VAR() calculaion rather than duplicating the basic logic... and also allow for the differences between Excel/Gnumeric and Open/LibreOffice
* Start implementing Newton-Raphson for the inverse of Statistical Distributions, starting with the two-tailed Student-T
* Additional unit tests and validations
* Use the new Newton Raphson class for calculating the Inverse of ChiSquared
* Extract Weibull distribution, and provide unit tests
* Extract ACCRINT() and ACCRINTM() Financial functions into their own class
Implement additional validations, with additional unit tests
Add support for the new calculation method argument for ACCRINT()
* Additional tests for Amortization functions
Continuing the process of breaking MathTrip.php up into smaller classes. This round takes care of all functions which might be an impediment to installing due to either uncovered code or "complexity":
- BASE
- FACT
- LCM
- MDETERM, MINVERSE, MMULT
- MULTINOMIAL
- PRODUCT
- QUOTIENT
- SERIESSUM
- SUM
- SUMPRODUCT
MathTrig and the members in directory MathTrig are now 100% covered. Many tests have been added, and some edge-case bugs are corrected. Some cases where PhpSpreadsheet had rejected numeric values stored as strings have been changed to accept them whenever Excel does; there had been no tests for that condition.
Boolean arguments are now accepted as arguments wherever Excel accpets them. Taking a cue from what has been done in Engineering, the parameter validation now happens in a routine which issues Exceptions for invalid values; this simplifies the code in the functions themselves. Thank you for doing that; I did not foresee how useful that was when I first looked at it.
Consistent with earlier changes of this nature, the versions in the MathTrig class remain, with a doc block indicating deprecation, and a stub call to the new routines.
All tests except for MINVERSE and MMULT are now handled in the context of a spreadsheet rather than a direct call to the calculation function which implements it. PhpSpreadsheet would need to handle dynamic arrays in order to test MINVERSE and MMULT in a spreadsheet context. Implementing that looks like it might be *very* challenging. It is not something I plan to look at, at least not in the near future.
One parsing problem turned up in the test conversion. It is in one of the SUMIF tests. It takes me to an area in Calculation where the comment says "I don't even want to know what you did to get here". It did not show up in the previous incarnation because, by using a direct call, the previous test managed to bypass the parsing. I have confirmed that this problem shows up in earlier releases of PhpSpreadsheet, so the changes in this PR did not cause it - they merely exposed it. I have left the test intact, but marked it "incomplete" for documentation purposes. I have not been able to get a handle on what's going wrong yet. I will probably open an issue on it if I can't resolve it soon. However, the test in question isn't a "real world" issue, and the error wasn't caused by this change, so I see no reason to delay this pending a resolution of the problem.
SUM had an idiosyncratic moment of its own. It had been ignoring non-numeric values, but Excel returns VALUE in that situation. So I changed it and wrote some new tests, which worked, but ... SUMIF uses several levels of indirection to get to SUM, and SUMIF *does* ignore non-numeric values, so a SUMIF test broke. SUM is a really simple function; the most practical approach seemed to be to clone it, with the string-accepting version being used by the Legacy version (which is called by SUMIF), and the non-string-accepting version being used in the Calculation Function table. That seems far easier and more practical than, for instance, adding a boolean parameter to the variable parameter list. As a follow-up, I will change SUMIF to explicitly call the appropriate new version, but I did not want to add that to this already large change.
SUM again - although it was fully covered beforehand, there was not a specific test member for it. There is now.
FACT had been coded to fail Gnumeric requests where the numeric argument has a decimal portion. However, Gnumeric does accept such an argument, and, unlike Excel and ODS, does not truncate it, but returns the result of a Gamma function call instead. This has been corrected.
When LCM included arguments which contained both 0 and a negative number, it returned 0 or NUM, whichever it found first. It is changed to always return NUM in that circumstance, as Excel does.
QUOTIENT had been documented as taking a variadic list of arguments. In fact, it takes exactly 2 - numerator and denominator - and the docblock and signature is fixed, even in the deprecated version.
The SERIESSUM docbock and signature are more accurate, even in the deprecated version. It is changed to ignore nulls, as Excel does, rather than return VALUE, and is one of the routines which previously rejected numbers in string form.
SUBTOTAL tests had used mocking for some reason. These are replaced with normal tests. And SUBTOTAL had a big surprise in store. That part of it which deals with hidden cells cares only whether the row is hidden, and doesn't care about the column's visibility.
I struggled with whether it should be SubTotal or Subtotal. I think the latter is correct, so that's how I proceeded. I don't think there are likely to be any other capitalization controversies.
* New Bessel Algorithm, providing a higher degree of precision (12 decimal places) and still matching/exceeding MS Excel's precision across the range of values
Mark Baker
oleibman
oleibman
Adrien Crivelli