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

Empty sparkline with floats #1137

Closed
anonymous-matomo-user opened this issue Feb 1, 2010 · 21 comments
Closed

Empty sparkline with floats #1137

anonymous-matomo-user opened this issue Feb 1, 2010 · 21 comments
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Milestone

Comments

@anonymous-matomo-user
Copy link

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.

@robocoder
Copy link
Contributor

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.

@anonymous-matomo-user
Copy link
Author

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?

@robocoder
Copy link
Contributor

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

@robocoder
Copy link
Contributor

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

@mattab
Copy link
Member

mattab commented Mar 4, 2010

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.

@mattab
Copy link
Member

mattab commented Mar 4, 2010

Attachment: Sparklines fail
sparkline fail.PNG

@pebosi
Copy link
Contributor

pebosi commented Mar 4, 2010

Hi, why not using http://omnipotent.net/jquery.sparkline/ ?

@robocoder
Copy link
Contributor

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.

@robocoder
Copy link
Contributor

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

@mattab
Copy link
Member

mattab commented Mar 5, 2010

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

@mattab
Copy link
Member

mattab commented Mar 5, 2010

Attachment:
index.php.png

@robocoder
Copy link
Contributor

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

@anonymous-matomo-user
Copy link
Author

Attachment: sparklines before hack
Capture-Piwik Web Analytics Reports - Mozilla Firefox.png

@anonymous-matomo-user
Copy link
Author

Attachment: sparklines after hack
Capture-Piwik Web Analytics Reports - Mozilla Firefox-1.png

@anonymous-matomo-user
Copy link
Author

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);

@anonymous-matomo-user
Copy link
Author

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);
}

@robocoder
Copy link
Contributor

(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

@anonymous-matomo-user
Copy link
Author

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...

@anonymous-matomo-user
Copy link
Author

Attachment: minimalistic debugging of feature point offset
patch-simple.txt

@anonymous-matomo-user
Copy link
Author

Attachment: debug of feature point offset and antialiasing
patch-antialias.txt

@robocoder
Copy link
Contributor

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

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Projects
None yet
Development

No branches or pull requests

4 participants