mitoteam / jpgraph

JpGraph library composer package with PHP 8.4 support
https://packagist.org/packages/mitoteam/jpgraph
Other
36 stars 5 forks source link

Warning Generating Pie3D #10

Closed oleibman closed 2 years ago

oleibman commented 2 years ago

This is the second problem I'm seeing. I am certain it is completely unrelated to the first problem. In this case, the output chart is generated and looks pretty good, but there is a warning message while it is being created:

[13-Sep-2022 08:51:50 America/Los_Angeles] PHP Warning:  A non-numeric value encountered in C:\git\bubble2prb\vendor\mitoteam\jpgraph\src\lib\jpgraph_pie3d.php on line 656

I can send the rest of the stack trace if you need it (it's pretty long). We have a similar problem with an Exploded Pie3D chart, but I assume the explanation is the same in either case. I am attaching a zip file containing the new spreadsheet, the new Php script, and the output file. You can just put the spreadsheet and the script in the same directory I sent yesterday and run the script to see the problem. Please let me know if there's anything we might do to avoid the warning message. pie3d.zip

oleibman commented 2 years ago

I think I have a line on this now. In jpgraph jpgraph_pie3d.php, in function setLabels, it initializes $this->ilabelposadj to "auto" if only one parameter is supplied. But, at line 655 (and later) it tries to use that property arithmetically. So perhaps "auto" is not a good default for that parameter (but I'm not sure how to report that problem). However, I can change my code to supply a numeric second parameter for setLabels (I think 1 is the appropriate value), and I think it will then work. Does that seem reasonable to you?

f1mishutka commented 2 years ago

Yes, you dug this to the right place.

But I don't like replacing 'auto' to 1.0 for whole run. This value being checked in some other places in pie3d and pie charts. So to make it safer I added patch to replace 'auto' value to 1.0 right in place where bug was. So you can safely leave default value for argument when initializing graph.

For you convenience I made 10.2.3 release with this patch. Please let me know if it works fine for you now.

Thank you!

P.S.:

but I'm not sure how to report that problem

This is one of main reasons why we decided to maintain library fork ourselves.

oleibman commented 2 years ago

I can confirm that 10.2.3 allows us to leave our invoking code unchanged. Thank you for the speedy resolution. Closing this issue now.