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

More Bubble Chart Fixes #2856

Merged
merged 2 commits into from
May 29, 2022
Merged

More Bubble Chart Fixes #2856

merged 2 commits into from
May 29, 2022

Conversation

oleibman
Copy link
Collaborator

Continuing the work from #2828, #2841, #2846, and #2852. This is probably my last change in this area for a while.

Bubble charts can have bubbles of different sizes. Phpspreadsheet had not supported this. Openpyxl comes with sample code to generate such a chart. I was especially drawn to that solution because its namespace usage would have been unexpected before 2852. And it turned out to come with other surprises - use of absolute paths in the .rels files (PhpSpreadsheet expected only relative), use of a one-cell anchor to place the chart (PhpSpreadsheet expected two-cell anchor or absolute positioning), plaintext in the legend (Phpspreadsheet expected RichText), no cached values for chart data. Excel handles the file okay, and this PR makes sure PhpSpreadsheet does as well. This file is now Samples/Templates/32readwriteBubbleChart2, and is used for both generating a sample output file and in formal tests. A new sample in the 33* series demonstrates how to code these.

This is:

- [x] a bugfix
- [ ] a new feature
- [ ] refactoring
- [ ] additional unit tests

Checklist:

Why this change is needed?

Provide an explanation of why this change is needed, with links to any Issues (if appropriate).
If this is a bugfix or a new feature, and there are no existing Issues, then please also create an issue that will make it easier to track progress with this PR.

Continuing the work from PHPOffice#2828, PHPOffice#2841, PHPOffice#2846, and PHPOffice#2852. This is probably my last change in this area for a while.

Bubble charts can have bubbles of different sizes. Phpspreadsheet had not supported this. Openpyxl comes with sample code to generate such a chart. I was especially drawn to that solution because its namespace usage would have been unexpected before 2852. And it turned out to come with other surprises - use of absolute paths in the .rels files (PhpSpreadsheet expected only relative), use of a one-cell anchor to place the chart (PhpSpreadsheet expected two-cell anchor or absolute positioning), plaintext in the legend (Phpspreadsheet expected RichText), no cached values for chart data. Excel handles the file okay, and this PR makes sure PhpSpreadsheet does as well. This file is now Samples/Templates/32readwriteBubbleChart2, and is used for both generating a sample output file and in formal tests. A new sample in the 33* series demonstrates how to code these.
@oleibman oleibman merged commit ab52991 into PHPOffice:master May 29, 2022
@oleibman oleibman deleted the bubblesize2 branch June 17, 2022 19:39
MarkBaker added a commit that referenced this pull request Jul 9, 2022
Note that this will be the last 1.x branch release before the 2.x release. We will maintain both branches in parallel for a time; but users are requested to update to version 2.0 once that is fully available.

### Added

- Added `removeComment()` method for Worksheet [PR #2875](https://github.com/PHPOffice/PhpSpreadsheet/pull/2875/files)
- Add point size option for scatter charts [Issue #2298](#2298) [PR #2801](#2801)
- Basic support for Xlsx reading/writing Chart Sheets [PR #2830](#2830)

  Note that a ChartSheet is still only written as a normal Worksheet containing a single chart, not as an actual ChartSheet.

- Added Worksheet visibility in Ods Reader [PR #2851](#2851) and Gnumeric Reader [PR #2853](#2853)
- Added Worksheet visibility in Ods Writer [PR #2850](#2850)
- Allow Csv Reader to treat string as contents of file [Issue #1285](#1285) [PR #2792](#2792)
- Allow Csv Reader to store null string rather than leave cell empty [Issue #2840](#2840) [PR #2842](#2842)
- Provide new Worksheet methods to identify if a row or column is "empty", making allowance for different definitions of "empty":
  - Treat rows/columns containing no cell records as empty (default)
  - Treat cells containing a null value as empty
  - Treat cells containing an empty string as empty

### Changed

- Modify `rangeBoundaries()`, `rangeDimension()` and `getRangeBoundaries()` Coordinate methods to work with row/column ranges as well as with cell ranges and cells [PR #2926](#2926)
- Better enforcement of value modification to match specified datatype when using `setValueExplicit()`
- Relax validation of merge cells to allow merge for a single cell reference [Issue #2776](#2776)
- Memory and speed improvements, particularly for the Cell Collection, and the Writers.

  See [the Discussion section on github](#2821) for details of performance across versions
- Improved performance for removing rows/columns from a worksheet

### Deprecated

- Nothing

### Removed

- Nothing

### Fixed

- Xls Reader resolving absolute named ranges to relative ranges [Issue #2826](#2826) [PR #2827](#2827)
- Null value handling in the Excel Math/Trig PRODUCT() function [Issue #2833](#2833) [PR #2834](#2834)
- Invalid Print Area defined in Xlsx corrupts internal storage of print area [Issue #2848](#2848) [PR #2849](#2849)
- Time interval formatting [Issue #2768](#2768) [PR #2772](#2772)
- Copy from Xls(x) to Html/Pdf loses drawings [PR #2788](#2788)
- Html Reader converting cell containing 0 to null string [Issue #2810](#2810) [PR #2813](#2813)
- Many fixes for Charts, especially, but not limited to, Scatter, Bubble, and Surface charts. [Issue #2762](#2762) [Issue #2299](#2299) [Issue #2700](#2700) [Issue #2817](#2817) [Issue #2763](#2763) [Issue #2219](#2219) [Issue #2863](#2863) [PR #2828](#2828) [PR #2841](#2841) [PR #2846](#2846) [PR #2852](#2852) [PR #2856](#2856) [PR #2865](#2865) [PR #2872](#2872) [PR #2879](#2879) [PR #2898](#2898) [PR #2906](#2906) [PR #2922](#2922) [PR #2923](#2923)
- Adjust both coordinates for two-cell anchors when rows/columns are added/deleted. [Issue #2908](#2908) [PR #2909](#2909)
- Keep calculated string results below 32K. [PR #2921](#2921)
- Filter out illegal Unicode char values FFFE/FFFF. [Issue #2897](#2897) [PR #2910](#2910)
- Better handling of REF errors and propagation of all errors in Calculation engine. [PR #2902](#2902)
- Calculating Engine regexp for Column/Row references when there are multiple quoted worksheet references in the formula [Issue #2874](#2874) [PR #2899](#2899)
@fredpe
Copy link

fredpe commented Dec 23, 2022

Leaving a note here while I am also preparing a new issue for a bug with a change made in this PR. We noticed this issue when upgrading to version 1.24.0 which included this PR here, and the problem is still present in the newest version 1.26.0. Running on 1.23.0, it is not showing this issue.

The error we are receiving after this PR, and which was not present before this PR with same inputs, is:

[Fri Dec 23 11:01:56.494355 2022] [php:notice] [pid 17] [client 172.16.0.243:21160] ErrorException: Warning: Undefined array key 0 in /quintly/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Writer/Xlsx/Chart.php:1398
Stack trace:
#0 /quintly/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Writer/Xlsx/Chart.php(1260): PhpOffice\\PhpSpreadsheet\\Writer\\Xlsx\\Chart->writePlotSeriesValues(Object(PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues), Object(PhpOffice\\PhpSpreadsheet\\Shared\\XMLWriter), 'barChart', 'str')
#1 /quintly/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Writer/Xlsx/Chart.php(273): PhpOffice\\PhpSpreadsheet\\Writer\\Xlsx\\Chart->writePlotGroup(Object(PhpOffice\\PhpSpreadsheet\\Chart\\DataSeries), 'barChart', Object(PhpOffice\\PhpSpreadsheet\\Shared\\XMLWriter), false, false, 'percentStacked')
#2 /quintly/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Writer/Xlsx/Chart.php(94): PhpOffice\\PhpSpreadsheet\\Writer\\Xlsx\\Chart->writePlotArea(Object(PhpOffice\\PhpSpreadsheet\\Shared\\XMLWriter), Object(PhpOffice\\PhpSpreadsheet\\Chart\\PlotArea), NULL, NULL, Object(PhpOffice\\PhpSpreadsheet\\Chart\\Axis), Object(PhpOffice\\PhpSpreadsheet\\Chart\\Axis))
#3 /quintly/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Writer/Xlsx.php(399): PhpOffice\\PhpSpreadsheet\\Writer\\Xlsx\\Chart->writeChart(Object(PhpOffice\\PhpSpreadsheet\\Chart\\Chart), false)
#4 /quintly/src/Metrics/Export/Office/Excel.php(277): PhpOffice\\PhpSpreadsheet\\Writer\\Xlsx->save('/tmp/reportsp16...')
#5 /quintly/src/Metrics/Export/Office/Excel.php(79): Metrics\\Export\\Office\\Excel->writeSheet()
#6 /quintly/src/Tool/View/Data/ExcelMetricsView.php(75): Metrics\\Export\\Office\\Excel->createReport(Array, 'MM/DD/YYYY')
#7 /quintly/src/Application/Tool/DataResponseRenderingService.php(64): Tool\\View\\Data\\ExcelMetricsView->render('B04 & FCB Repor...', Array, Object(Metrics\\UserSettings))
#8 /quintly/src/Tool/Controller/DataController.php(176): Application\\Tool\\DataResponseRenderingService->getResponseByFormat('xlsx', Object(Metrics\\ExportContext), Array, Object(Metrics\\UserSettings), Object(Domain\\Customer\\Entity\\User), Object(DoctrineProxies\\__CG__\\Domain\\Customer\\Entity\\Space))
#9 /quintly/src/Common/Foundation/AbstractController.php(64): Tool\\Controller\\DataController->loadDashboardMetricsAction()
#10 /quintly/projects/tool/public/index-zend.php(53): Common\\Foundation\\AbstractController->callAction('loadDashboardMe...')
#11 {main}, referer: [...]

It is related to the change in this PR to this line:
https://github.com/PHPOffice/PhpSpreadsheet/pull/2856/files#diff-97c03ccbe2f3cac6b5390cd2c45c1a12d6786c13851ea9cbe74416cc2db8b066R1445

if ($count > 1 || ($count === 1 && "=$source" !== (string) $values[0])) {

The issue is caused because the point count ($plotSeriesValues->getPointCount()) is out of sync with the number of data values ($plotSeriesValues->getDataValues()). Our log output shows that the point count is 1, while the data values is an empty array. As a result, the mentioned exception is thrown. Here is a print_r of the PhpOffice\PhpSpreadsheet\Chart\DataSeriesValues object when the issue occurs:

PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues Object
(
    [objectState:protected] => 
    [glowSize:protected] => 
    [glowColor:protected] => PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor Object
        (
            [value:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
            [type:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
            [alpha:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
            [brightness:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
        )

    [softEdges:protected] => Array
        (
            [size] => 
        )

    [shadowProperties:protected] => Array
        (
            [presets] => 
            [effect] => 
            [size] => Array
                (
                    [sx] => 
                    [sy] => 
                    [kx] => 
                    [ky] => 
                )

            [blur] => 
            [direction] => 
            [distance] => 
            [algn] => 
            [rotWithShape] => 
        )

    [shadowColor:protected] => PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor Object
        (
            [value:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => black
            [type:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => prstClr
            [alpha:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 40
            [brightness:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
        )

    [lineColor:protected] => PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor Object
        (
            [value:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
            [type:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
            [alpha:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
            [brightness:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
        )

    [lineStyleProperties:protected] => Array
        (
            [width] => 
            [compound] => 
            [dash] => 
            [cap] => 
            [join] => 
            [arrow] => Array

   (
                    [head] => Array
                        (
                            [type] => 
                            [size] => 
                            [w] => 
                            [len] => 
                        )

                    [end] => Array
                        (
                            [type] => 

            [size] => 
                            [w] => 
                            [len] => 
                        )

                )

        )

    [dataType:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => String
    [dataSource:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => 'Instagram Interactions Distribu'!$A$4:$A$4
    [formatCode:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => 
    [pointMarker:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => 
    [markerFillColor:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor Object
        (
            [value:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
            [type:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
            [alpha:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
            [brightness:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
        )

    [markerBorderColor:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor Object
        (
            [value:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
            [type:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
            [alpha:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
            [brightness:PhpOffice\\PhpSpreadsheet\\Chart\\ChartColor:private] => 
        )

    [pointSize:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => 3
    [pointCount:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => 1
    [dataValues:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => Array
        (
        )

    [fillColor:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => 
    [scatterLines:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => 1
    [bubble3D:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => 
    [labelLayout:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => 
    [trendLines:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => Array
        (
        )

    [smoothLine:PhpOffice\\PhpSpreadsheet\\Chart\\DataSeriesValues:private] => 
)

Again, I am sorry I am not handing in an isolated test case yet, which I am happy to provide in a separate bug issue, but it will require some more preparation. I still thought probably someone more into these objects immediately sees why this "out of sync" is happening, or if it's even valid and the check should rather be on count($values) === 1 instead of $count === 1 before the array access with [0] is happening.

@oleibman
Copy link
Collaborator Author

Thank you for the excellent documentation. As you mentioned, please open a new issue. If possible, there is something I would like you to try. The statement which you have identified is:

if ($count > 1 || ($count === 1 && "=$source" !== (string) $values[0])) {

If you change that to:

if ($count > 1 || ($count === 1 && array_key_exists(0, $values) && "=$source" !== (string) $values[0])) {

I think that will eliminate the error, but it may just push the error along to somewhere further in the program, or it may result in a corrupt or incorrect spreadsheet. I cannot run this test on my own since I do not yet have an example of a failing worksheet. Can you test this and see what happens.

@fredpe
Copy link

fredpe commented Dec 28, 2022

Thank you @oleibman for the very prompt feedback! I have tested the patch you described, and it indeed resolves the error, but the output is not the same anymore, even though the difference is little.

In the following screenshot you see the exact same code with exact same inputs running on 1.23.0 on the left, and 1.26.0 with the array_key_exists patch on the right. As you can see, the charts have the same cells as an input, but render slightly different.

image

Now I am not sure, if this difference in rendering is simply a change that came with other updates since 1.23.0, as also the logic producing the error was just introduced in 1.24.0 and did not exist before. At least I can say, other than formatting differences in the charts, the data inputs and also the construction of the chart itself with its parameters is still correct.

Hope this helps! I will work on the isolated test case in the meanwhile.

@oleibman
Copy link
Collaborator Author

Glad that we're on the right track even if we don't have a complete solution yet. I don't think I can do much more without a sample input file and/or program in hand.

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this pull request Feb 28, 2023
See discussion at bottom of PR PHPOffice#2856.
oleibman added a commit that referenced this pull request Mar 2, 2023
* Misplaced Xml Writing Chart Label FillColor

Fix #3397. Move code to correct location.

* Update Issue3397Test.php

No need for spacing patch - just fix it now.

* Minor Fix

See discussion at bottom of PR #2856.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants