From e496d856c0278d976747005a7ed64ef2d447256f Mon Sep 17 00:00:00 2001 From: Amornthep <93817415+quonly@users.noreply.github.com> Date: Mon, 3 Feb 2025 14:13:40 +0700 Subject: [PATCH 1/5] change $trenType to $trendMethod in case TREND_BEST_FIT --- src/PhpSpreadsheet/Shared/Trend/Trend.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/Shared/Trend/Trend.php b/src/PhpSpreadsheet/Shared/Trend/Trend.php index dc8794300d..2616574d29 100644 --- a/src/PhpSpreadsheet/Shared/Trend/Trend.php +++ b/src/PhpSpreadsheet/Shared/Trend/Trend.php @@ -94,7 +94,7 @@ public static function calculate(string $trendType = self::TREND_BEST_FIT, array $bestFit = []; $bestFitValue = []; foreach (self::$trendTypes as $trendMethod) { - $className = '\PhpOffice\PhpSpreadsheet\Shared\Trend\\' . $trendType . 'BestFit'; + $className = '\PhpOffice\PhpSpreadsheet\Shared\Trend\\' . $trendMethod . 'BestFit'; //* @phpstan-ignore-next-line $bestFit[$trendMethod] = new $className($yValues, $xValues, $const); $bestFitValue[$trendMethod] = $bestFit[$trendMethod]->getGoodnessOfFit(); From 263a397b847f0043d0ca6e1ad1345e3c4bb754a5 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sun, 9 Mar 2025 20:21:16 -0700 Subject: [PATCH 2/5] Add test, Disable TREND_BEST_FIT, POLYNOMIAL_BEST_FIT --- .../Shared/Trend/PolynomialBestFit.php | 9 +++ src/PhpSpreadsheet/Shared/Trend/Trend.php | 9 +-- .../Shared/Trend/BestFitTest.php | 72 +++++++++++++++++++ 3 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Shared/Trend/BestFitTest.php diff --git a/src/PhpSpreadsheet/Shared/Trend/PolynomialBestFit.php b/src/PhpSpreadsheet/Shared/Trend/PolynomialBestFit.php index 188c2cedb5..955949d42c 100644 --- a/src/PhpSpreadsheet/Shared/Trend/PolynomialBestFit.php +++ b/src/PhpSpreadsheet/Shared/Trend/PolynomialBestFit.php @@ -3,11 +3,14 @@ namespace PhpOffice\PhpSpreadsheet\Shared\Trend; use Matrix\Matrix; +use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException; // Phpstan and Scrutinizer seem to have legitimate complaints. // $this->slope is specified where an array is expected in several places. // But it seems that it should always be float. // This code is probably not exercised at all in unit tests. +// Private bool property $implemented is set to indicate +// whether this implementation is correct. class PolynomialBestFit extends BestFit { /** @@ -21,6 +24,8 @@ class PolynomialBestFit extends BestFit */ protected int $order = 0; + private bool $implemented = false; + /** * Return the order of this polynomial. */ @@ -187,6 +192,10 @@ private function polynomialRegression(int $order, array $yValues, array $xValues */ public function __construct(int $order, array $yValues, array $xValues = []) { + if (!$this->implemented) { + throw new SpreadsheetException('Polynomial Best Fit not yet implemented'); + } + parent::__construct($yValues, $xValues); if (!$this->error) { diff --git a/src/PhpSpreadsheet/Shared/Trend/Trend.php b/src/PhpSpreadsheet/Shared/Trend/Trend.php index 2616574d29..60a588524c 100644 --- a/src/PhpSpreadsheet/Shared/Trend/Trend.php +++ b/src/PhpSpreadsheet/Shared/Trend/Trend.php @@ -18,10 +18,8 @@ class Trend /** * Names of the best-fit Trend analysis methods. - * - * @var string[] */ - private static array $trendTypes = [ + private const TREND_TYPES = [ self::TREND_LINEAR, self::TREND_LOGARITHMIC, self::TREND_EXPONENTIAL, @@ -93,13 +91,12 @@ public static function calculate(string $trendType = self::TREND_BEST_FIT, array // Start by generating an instance of each available Trend method $bestFit = []; $bestFitValue = []; - foreach (self::$trendTypes as $trendMethod) { + foreach (self::TREND_TYPES as $trendMethod) { $className = '\PhpOffice\PhpSpreadsheet\Shared\Trend\\' . $trendMethod . 'BestFit'; - //* @phpstan-ignore-next-line $bestFit[$trendMethod] = new $className($yValues, $xValues, $const); $bestFitValue[$trendMethod] = $bestFit[$trendMethod]->getGoodnessOfFit(); } - if ($trendType != self::TREND_BEST_FIT_NO_POLY) { + if ($trendType !== self::TREND_BEST_FIT_NO_POLY) { foreach (self::$trendTypePolynomialOrders as $trendMethod) { $order = (int) substr($trendMethod, -1); $bestFit[$trendMethod] = new PolynomialBestFit($order, $yValues, $xValues); diff --git a/tests/PhpSpreadsheetTests/Shared/Trend/BestFitTest.php b/tests/PhpSpreadsheetTests/Shared/Trend/BestFitTest.php new file mode 100644 index 0000000000..c2fc21db18 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Shared/Trend/BestFitTest.php @@ -0,0 +1,72 @@ +getGoodnessOfFit(); + if ($maxGoodness < $goodness) { + $maxGoodness = $goodness; + $maxType = $type; + } + self::assertEqualsWithDelta(0.9628, $goodness, self::LBF_PRECISION); + + $type = Trend::TREND_EXPONENTIAL; + $result = Trend::calculate($type, $yValues, $xValues); + $goodness = $result->getGoodnessOfFit(); + if ($maxGoodness < $goodness) { + $maxGoodness = $goodness; + $maxType = $type; + } + self::assertEqualsWithDelta(0.9952, $goodness, self::LBF_PRECISION); + + $type = Trend::TREND_LOGARITHMIC; + $result = Trend::calculate($type, $yValues, $xValues); + $goodness = $result->getGoodnessOfFit(); + if ($maxGoodness < $goodness) { + $maxGoodness = $goodness; + $maxType = $type; + } + self::assertEqualsWithDelta(-0.0724, $goodness, self::LBF_PRECISION); + + $type = Trend::TREND_POWER; + $result = Trend::calculate($type, $yValues, $xValues); + $goodness = $result->getGoodnessOfFit(); + if ($maxGoodness < $goodness) { + $maxGoodness = $goodness; + $maxType = $type; + } + self::assertEqualsWithDelta(0.9946, $goodness, self::LBF_PRECISION); + + $type = Trend::TREND_BEST_FIT_NO_POLY; + $result = Trend::calculate($type, $yValues, $xValues); + $goodness = $result->getGoodnessOfFit(); + self::assertSame($maxGoodness, $goodness); + self::assertSame(lcfirst($maxType), $result->getBestFitType()); + + try { + $type = Trend::TREND_BEST_FIT; + $result = Trend::calculate($type, $yValues, $xValues); + self::fail('should have failed - TREND_BEST_FIT includes polynomials which are not implemented yet'); + } catch (SpreadsheetException $e) { + self::assertStringContainsString('not yet implemented', $e->getMessage()); + } + } +} From d462fbf05ef955fa8bd295c3c4d1a79268da3209 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sun, 9 Mar 2025 21:27:29 -0700 Subject: [PATCH 3/5] Fix Phpstan and Scrutinizer Problems --- src/PhpSpreadsheet/Shared/Trend/Trend.php | 8 +++++--- .../Shared/Trend/BestFitTest.php | 18 +++++++++++++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/PhpSpreadsheet/Shared/Trend/Trend.php b/src/PhpSpreadsheet/Shared/Trend/Trend.php index 60a588524c..bfd9125ed1 100644 --- a/src/PhpSpreadsheet/Shared/Trend/Trend.php +++ b/src/PhpSpreadsheet/Shared/Trend/Trend.php @@ -2,6 +2,8 @@ namespace PhpOffice\PhpSpreadsheet\Shared\Trend; +use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException; + class Trend { const TREND_LINEAR = 'Linear'; @@ -46,7 +48,7 @@ class Trend */ private static array $trendCache = []; - public static function calculate(string $trendType = self::TREND_BEST_FIT, array $yValues = [], array $xValues = [], bool $const = true): mixed + public static function calculate(string $trendType = self::TREND_BEST_FIT, array $yValues = [], array $xValues = [], bool $const = true): BestFit { // Calculate number of points in each dataset $nY = count($yValues); @@ -57,7 +59,7 @@ public static function calculate(string $trendType = self::TREND_BEST_FIT, array $xValues = range(1, $nY); } elseif ($nY !== $nX) { // Ensure both arrays of points are the same size - trigger_error('Trend(): Number of elements in coordinate arrays do not match.', E_USER_ERROR); + throw new SpreadsheetException('Trend(): Number of elements in coordinate arrays do not match.'); } $key = md5($trendType . $const . serialize($yValues) . serialize($xValues)); @@ -113,7 +115,7 @@ public static function calculate(string $trendType = self::TREND_BEST_FIT, array return $bestFit[$bestFitType]; default: - return false; + throw new SpreadsheetException("Unknown trend type $trendType"); } } } diff --git a/tests/PhpSpreadsheetTests/Shared/Trend/BestFitTest.php b/tests/PhpSpreadsheetTests/Shared/Trend/BestFitTest.php index c2fc21db18..4feb8d60ac 100644 --- a/tests/PhpSpreadsheetTests/Shared/Trend/BestFitTest.php +++ b/tests/PhpSpreadsheetTests/Shared/Trend/BestFitTest.php @@ -63,10 +63,26 @@ public function testBestFit(): void try { $type = Trend::TREND_BEST_FIT; - $result = Trend::calculate($type, $yValues, $xValues); + Trend::calculate($type, $yValues, [0, 1, 2]); + self::fail('should have failed - mismatched number of elements'); + } catch (SpreadsheetException $e) { + self::assertStringContainsString('Number of elements', $e->getMessage()); + } + + try { + $type = Trend::TREND_BEST_FIT; + Trend::calculate($type, $yValues, $xValues); self::fail('should have failed - TREND_BEST_FIT includes polynomials which are not implemented yet'); } catch (SpreadsheetException $e) { self::assertStringContainsString('not yet implemented', $e->getMessage()); } + + try { + $type = 'unknown'; + Trend::calculate($type, $yValues, $xValues); + self::fail('should have failed - invalid trend type'); + } catch (SpreadsheetException $e) { + self::assertStringContainsString('Unknown trend type', $e->getMessage()); + } } } From 6e1f3b909c241b40cf5843e39aa9bacec05d21cd Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sun, 9 Mar 2025 21:56:32 -0700 Subject: [PATCH 4/5] More Phpstan Having Trend:calculate return BestFit rather than mixed eliminates many Phpstan messages in baseline.neon. --- phpstan-baseline.neon | 84 ------------------------------------------- 1 file changed, 84 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 9530d4e71e..fe7b6960b0 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1434,90 +1434,6 @@ parameters: count: 1 path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - message: '#^Cannot call method getCorrelation\(\) on mixed\.$#' - identifier: method.nonObject - count: 1 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getCovariance\(\) on mixed\.$#' - identifier: method.nonObject - count: 1 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getDFResiduals\(\) on mixed\.$#' - identifier: method.nonObject - count: 2 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getF\(\) on mixed\.$#' - identifier: method.nonObject - count: 2 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getGoodnessOfFit\(\) on mixed\.$#' - identifier: method.nonObject - count: 3 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getIntersect\(\) on mixed\.$#' - identifier: method.nonObject - count: 5 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getIntersectSE\(\) on mixed\.$#' - identifier: method.nonObject - count: 2 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getSSRegression\(\) on mixed\.$#' - identifier: method.nonObject - count: 2 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getSSResiduals\(\) on mixed\.$#' - identifier: method.nonObject - count: 2 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getSlope\(\) on mixed\.$#' - identifier: method.nonObject - count: 5 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getSlopeSE\(\) on mixed\.$#' - identifier: method.nonObject - count: 2 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getStdevOfResiduals\(\) on mixed\.$#' - identifier: method.nonObject - count: 3 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getValueOfYForX\(\) on mixed\.$#' - identifier: method.nonObject - count: 3 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - - - message: '#^Cannot call method getXValues\(\) on mixed\.$#' - identifier: method.nonObject - count: 2 - path: src/PhpSpreadsheet/Calculation/Statistical/Trends.php - - message: '#^Parameter \#1 \$yValues of static method PhpOffice\\PhpSpreadsheet\\Calculation\\Statistical\\Trends\:\:validateTrendArrays\(\) expects array, mixed given\.$#' identifier: argument.type From 8f3bb96387976fb0df5784d5716ecafee26d83eb Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Tue, 11 Mar 2025 23:45:31 -0700 Subject: [PATCH 5/5] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a567e36e68..d589454b4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Phpstan Version 2. [PR #4384](https://github.com/PHPOffice/PhpSpreadsheet/pull/4384) - Start migration to Phpstan level 9. [PR #4396](https://github.com/PHPOffice/PhpSpreadsheet/pull/4396) +- TREND_POLYNOMIAL_* and TREND_BEST_FIT do not work, and are changed to throw Exceptions if attempted. (TREND_BEST_FIT_NO_POLY works.) An attempt to use an unknown trend type will now also throw an exception. [Issue #4400](https://github.com/PHPOffice/PhpSpreadsheet/issues/4400) [PR #4339](https://github.com/PHPOffice/PhpSpreadsheet/pull/4339) ### Moved @@ -32,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed - BIN2DEC, OCT2DEC, and HEX2DEC return numbers rather than strings. [Issue #4383](https://github.com/PHPOffice/PhpSpreadsheet/issues/4383) [PR #4389](https://github.com/PHPOffice/PhpSpreadsheet/pull/4389) +- Fix TREND_BEST_FIT_NO_POLY. [Issue #4400](https://github.com/PHPOffice/PhpSpreadsheet/issues/4400) [PR #4339](https://github.com/PHPOffice/PhpSpreadsheet/pull/4339) ## 2025-03-02 - 4.1.0