Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#1137 closed Bug (fixed)

Empty sparkline with floats

Reported by: lmeyer Owned by: vipsoft
Priority: major Milestone: Piwik 0.6
Component: Core Keywords:
Cc: Sensitive: no

Description

The values of the Goal Plugin are floats (e.g. 1.53% conversions).
The sparkline of conversion rate and eventually overall revenue do not show up.

Correction: I got all sparklines showing up after this hack that converts all values to integers:
/core/Visualization/Sparkline.php method Piwik_Visualization_Sparkline->main() line 67, I add this after the foreach statement:
$value = (int)($value*100);

The factor 100 does no harm to all other sparklines, because they are resized. I would be good to have this included in the next release.

Attachments (6)

sparkline fail.PNG (10.9 KB) - added by matt 4 years ago.
Sparklines fail
index.php.png (1.1 KB) - added by matt 4 years ago.
Capture-Piwik › Web Analytics Reports - Mozilla Firefox.png (110.0 KB) - added by lmeyer 4 years ago.
sparklines before hack
Capture-Piwik › Web Analytics Reports - Mozilla Firefox-1.png (112.0 KB) - added by lmeyer 4 years ago.
sparklines after hack
patch-simple.txt (1.8 KB) - added by lmeyer 4 years ago.
minimalistic debugging of feature point offset
patch-antialias.txt (6.1 KB) - added by lmeyer 4 years ago.
debug of feature point offset and antialiasing

Download all attachments as: .zip

Change History (25)

comment:1 Changed 4 years ago by vipsoft (robocoder)

No, we already scale conversion ratios by 100. This is a bug in the sparkline-php library.

I'll sync up and see if a fix is still needed.

comment:2 Changed 4 years ago by lmeyer

I var_dumped $this->data, and it was not scaled by 100. Are you sure this applies to Goal tracking too?
Do you convert (float) to (int) to work around the bug in sparkline-php?

comment:3 Changed 4 years ago by vipsoft (robocoder)

Yes, I'm sure. The scaling is applied when the conversion rate is calculated and stored in the database.

comment:4 Changed 4 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from new to closed

(In [1822]) fixes #1137 - empty sparkline with floats

comment:5 Changed 4 years ago by matt (mattab)

  • Resolution fixed deleted
  • Status changed from closed to reopened

I svn up, and generated sparklines, but sparklines don't look good. I think that is how they used to look like before I manually patched the library, but the changes in [1822] probably reverted this.

See the attached screenshot. The points are many pixels off on the left which looks confusing.

Changed 4 years ago by matt (mattab)

Sparklines fail

comment:6 Changed 4 years ago by vipsoft (robocoder)

  • Owner set to vipsoft
  • Status changed from reopened to new

comment:7 Changed 4 years ago by pebosi

comment:8 Changed 4 years ago by vipsoft (robocoder)

pebosi: yes, I've already looked at that. At this time, I'm not going to change that.

Looking down the road, we might want to support both client and server-side chart generation:

  • Client: to offload processing from the server and loosen the installation requirements (gd).
  • Server: for email/pdf reports.

comment:9 Changed 4 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from new to closed

(In [1884]) fixes #1137 - off-by-one in x-axis range

comment:10 Changed 4 years ago by matt (mattab)

thanks Anthon. There is still a small offset in the green point but I think this has always been the case (see attached sparkline)

Changed 4 years ago by matt (mattab)

comment:11 Changed 4 years ago by vipsoft (robocoder)

Near as I can tell, it's a scaling artifact. Sometimes noticeable, sometimes not.

Changed 4 years ago by lmeyer

sparklines before hack

Changed 4 years ago by lmeyer

sparklines after hack

comment:12 Changed 4 years ago by lmeyer

The problem I initially reported is still not corrected in piwik 0.5.5, as the two uploaded screenshots prove.
The first shows piwik 0.5.5 as is,
the second shows the same statistics after I reapplied the patch described above:
/core/Visualization/Sparkline.php method Piwik_Visualization_Sparkline->main() line 67, I add this after the foreach statement:
$value = (int)($value*100);

comment:13 Changed 4 years ago by matt (mattab)

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:14 Changed 4 years ago by matt (mattab)

  • Milestone changed from Piwik 0.5.5 to 1 - Piwik 0.6

comment:15 Changed 4 years ago by vipsoft (robocoder)

  • Status changed from reopened to new

comment:16 Changed 4 years ago by lmeyer

The bug is in the sparkline library itself. The sparkline library is converting all input first into a string, and then test if it is numeric.
According to http://ch.php.net/manual/fr/language.types.float.php#language.types.float.casting, php is using the decimale separator defined in the current locale when converting to string, but only recognizes '.' as decimal separator while testing is_numeric.
My backend is configured in the locale fr_FR, which means that (string)1.5 converts to '1,5'.

The solution is to comment out the first two lines in libs/sparkline/lib/Sparkline_Line.php of the method SetData:
$x = trim($x);
$y = trim($y);
or to change them to (which will be a more general solution):
if(!is_numeric($x)){

$x = trim($x);

}
if(!is_numeric($y)){

$y = trim($y);

}

comment:17 Changed 4 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from new to closed

(In [1982]) fixes #1137 - thanks Lorenz (lmeyer) to debugging this locale issue; reported upstream in https://sourceforge.net/tracker/?func=detail&aid=2975415&group_id=122936&atid=694962

comment:18 Changed 4 years ago by lmeyer

I created two patches that correct the offset issue.
The first patch (patch-simple.txt) only corrects the bug in the sparkline lib (and your workaround in Sparkline.php)
The second patch (patch-antialias.txt) pretty much rewrites $Sparkline->RenderResampled and also antialiases the feature points.
I like real circles better than the losanges of piwik sparklines, but it is up to you to decide...

Changed 4 years ago by lmeyer

minimalistic debugging of feature point offset

Changed 4 years ago by lmeyer

debug of feature point offset and antialiasing

comment:19 Changed 4 years ago by vipsoft (robocoder)

(In [1983]) refs #1137 - fixing various off-by-one errors in drawing feature points; credit: lmeyer

Note: See TracTickets for help on using tickets.