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

[ISSUE 3] Inconsistencies with fractions and percentages #82

Open
anthonyfok opened this issue Oct 14, 2022 · 9 comments
Open

[ISSUE 3] Inconsistencies with fractions and percentages #82

anthonyfok opened this issue Oct 14, 2022 · 9 comments
Assignees
Labels
Bug Something isn't working Question

Comments

@anthonyfok
Copy link
Member

From @tieganh:

Generally speaking, we need to be consistent with presenting things as fractions or as percentages, and not use the “%” on fractions. The example shown has a hover over value of “0.49%”, but the legend hover over for this shade of red is “44.44+ %“. If we are multiplying legend values by 100 and adding the “%”, then we should do the same for the hover over, detail box, and left hand table. Probably easier at this stage to omit the “%” and change legend values to fractions?

Screen Shot 2022-10-13 at 7 15 33 PM

@anthonyfok anthonyfok added the Bug Something isn't working label Oct 14, 2022
@anthonyfok anthonyfok added this to Backlog in RiskProfiler via automation Oct 14, 2022
@anthonyfok anthonyfok moved this from Backlog to In progress in RiskProfiler Oct 14, 2022
@anthonyfok
Copy link
Member Author

@phil-evans replies:

i think this is also related to the changes to the rounding rules - this page was initially set up so that the returned data would follow the rounding rules that are created for the indicator in WP, but it seems to have lost that step after we added the new formatting logic. i’ll see if i can find where that’s going wrong

@anthonyfok
Copy link
Member Author

@jvanulde replies:

I think easiest to multiply the hover over values by 100, and redo the legend to align with this maximum.
@plesueur?

@anthonyfok
Copy link
Member Author

@plesueur replies:

We should stay consistent with what we're presenting on the sidebar. For the 'probability of building damage' case, we use fractions and not a percent. So I agree with @tieganh to omit the % and change the legend values to fractions.

@anthonyfok
Copy link
Member Author

@phil-evans asks:

just to confirm - for all of the ‘probability’ indicators, i should remove the % symbols from the popup and legend?

Screen Shot 2022-10-14 at 2 03 58 PM

@anthonyfok
Copy link
Member Author

@plesueur explains:

Follow the units displayed on the side panel. In other words:

  • Anything related to the 'Probability of X building damage over 50 years', remove the % symbols from the map and legend (and the values in the legend should not be percentages either).

  • The 'annual economic loss ratio', should remain as a percent, and the map pop-up should be a percent as well. Right now I think its not given as a percent, so please multiply this value by 100 so it matches the sidepanel (see screen shot below).

  • The 'annual probability of fatality' is not represented as a % on the map or side panel (no changes needed for this one, just including for completeness)

image

@anthonyfok
Copy link
Member Author

@phil-evans replies:

ok, i believe this is all right now:

Screen Shot 2022-10-14 at 3 15 41 PM

@anthonyfok
Copy link
Member Author

@phil-evans asks:

one thing i noticed - the annual economic loss ratio in the sidebar is pulling eAALm_Bldg, not eAALm_Asset.. not sure if that’s right or not

@wkhchow
Copy link
Contributor

wkhchow commented Oct 17, 2022

@phil-evans asks:

one thing i noticed - the annual economic loss ratio in the sidebar is pulling eAALm_Bldg, not eAALm_Asset.. not sure if that’s right or not

If that's the case I believe we want it to match asset for all, so annual economic loss ratio should b e pointing to eAALm_Asset and not eAALm_Bldg, which in the case for North Vancouver above should be 0.044% @plesueur

@plesueur
Copy link

yep, @wkhchow has got it right. Use eAALm_Asset.

Good catch @phil-evans

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Question
Projects
RiskProfiler
In progress
Development

No branches or pull requests

4 participants