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

Improve handling/formatting of percent values in Matomo #20701

Open
sgiehl opened this issue May 11, 2023 · 7 comments
Open

Improve handling/formatting of percent values in Matomo #20701

sgiehl opened this issue May 11, 2023 · 7 comments
Labels
c: APIs For bugs and features in the Matomo HTTP and plugin APIs. c: Data Integrity & Accuracy c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Technical debt Issues the will help to reduce technical debt

Comments

@sgiehl
Copy link
Member

sgiehl commented May 11, 2023

Matomo stores values that are displayed as percent values as rates.
So a value like 12.345% would be stored as 0.12345 in the database.

To receive the percent value out of the rate all percent metric classes are currently using the format method. e.g.

public function format($value, Formatter $formatter)
{
return $formatter->getPrettyPercentFromQuotient($value);
}

public function getPrettyPercentFromQuotient($value)
{
return NumberFormatter::getInstance()->formatPercent($value * 100, 4, 0);
}

This works perfectly when formatting results for the API. But in our UI there are several places where we actually can't directly work with formatted numbers, as we need to apply mathematical operations. This currently produces problems as when fetching unformatted values from the API the rate value would be returned. But as the UI doesn't know that the value would need to be calculated before formatting, we can't use it unformatted. In the past we have implemented various workarounds.

The biggest example would be the API parameter format_metrics=bc, which actually does not format anything except of percent values and is used as default for formatting. That way the percent values are correctly calculated but also formatted. If those numbers then need to be used for calculation this can result in a problem for certain languages. While the English presentation like 14.55% can be easily converted to float, this doesn't apply for e.g. the German one 14,55% or the Turkish %14,55. Both might end up in invalid/incorrect numbers.

There might be various ways to fix this. We might need to discuss the possible solutions to find a proper way, that doesn't end up in another workaround that we might need to refactor at some point.

My ideas for a possible solution would be:

  • Change all rate/percent metric classes so they do the calculation (multiply with 100) within the compute method (or similar), so the formatting method only applies the formatting.

    This might actually be some sort of breaking change, as the API would no longer return rate values when data is fetched unformatted. But we would be able to get rid of the bc formatting, and UI would be able to calculate with all values.

  • Let the UI know which values are returned as rates, so the UI can calculate the percent value when unformatted values are fetched

    This would be a quite big change, as currently the UI does not know which types the metrics are of. But if it's on purpose to return rate values and we can't change that, this might be an alternative solution.

refs #19940

@sgiehl sgiehl added Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. labels May 11, 2023
@sgiehl sgiehl added this to the For Prioritization milestone May 11, 2023
@mattab mattab modified the milestones: For Prioritization, 5.4.0 Jun 13, 2023
@sgiehl
Copy link
Member Author

sgiehl commented Jun 16, 2023

Had some more thoughts on the way of solving the formatting issues.

I might no longer go with the solution of changing how percent values are formatted in backend/API.

Instead we should aim to adjust our API, so unformatted responses contains additional metadata about the metric types, so it's possible to format them based on that.
We already have added some metadata for looker studio. Maybe those data could be re-used for that purpose as well.

After implementing this and migrating all places our UI should fully work with unformatted values.

So the proposed steps to ensure a correct handling of percent values would be:

@ThorstenMichel
Copy link

ThorstenMichel commented Aug 9, 2023

There is even a bug regarding Turkish percentages, I'm not sure if I shall open a new bug ticket for that:

Steps to reproduce

  • Change UI language to Türkçe
  • Open Behavior

Results

  • If bounce rate = 0%, it is correct displayed as: %0 (as usual in Turkiye)
  • If bounce rate >= 0%, it is displayed as all kind of letters; it seems %n is interpreted as entities
    image

@sgiehl
Copy link
Member Author

sgiehl commented Aug 9, 2023

@ThorstenMichel Thanks for reporting this one. There hadn't been any report yet that percent values in tables aren't displayed at all for Turkish. I found the problem, but not yet sure how to solve it best.

@diosmosis diosmosis self-assigned this Aug 10, 2023
@diosmosis
Copy link
Member

@sgiehl fyi, @mattab asked me to look into this for the moment. I'll put up an incomplete PR soon-ish if you'd like to look at the implementation I'm going w/ early.

@sgiehl
Copy link
Member Author

sgiehl commented Aug 11, 2023

@diosmosis the core team is currently building up a vision how metrics and their formatting should look like in long term. We are not yet at a point where we could share any details, but if your changes aren't in line with the approach we establish we might not be able to merge your contribution or need to rewrite it afterwards again, unnecessarily increasing the work for everyone.

@diosmosis
Copy link
Member

diosmosis commented Aug 11, 2023

@sgiehl here is the solution I am working towards:

does this approach have a problem in it? In the meantime, I will try to speak to @mattab about this as he chose the issue as one of the ones he'd like me to work on.

@sgiehl sgiehl added the Technical debt Issues the will help to reduce technical debt label Aug 28, 2023
@matomo-org matomo-org deleted a comment from diosmosis Dec 10, 2023
@mattab mattab added c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. c: APIs For bugs and features in the Matomo HTTP and plugin APIs. c: Data Integrity & Accuracy labels Dec 10, 2023
@diosmosis diosmosis removed their assignment Dec 10, 2023
@mattab mattab modified the milestones: 5.4.0, 5.5.0 Dec 14, 2023
@paul-buttimore
Copy link

We also have a customer who has noticed the video play rate in the evolution report is displayed as 0.27% instead of 27% as shown in the media titles report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: APIs For bugs and features in the Matomo HTTP and plugin APIs. c: Data Integrity & Accuracy c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Technical debt Issues the will help to reduce technical debt
Projects
None yet
Development

No branches or pull requests

6 participants