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

fix: don't excessively shorten_number (backport #26760) #26763

Merged

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Jun 14, 2024

Reproduce

Create two Number Cards, "With decimals" and "Without decimals", calling these two custom methods:

import frappe


@frappe.whitelist()
def with_decimals(filters=None):
	return {
		"value": 1234.56789,
		"fieldtype": "Float",
	}


@frappe.whitelist()
def without_decimals(filters=None):
	return {
		"value": 1234.0,
		"fieldtype": "Float",
	}

Before

Bildschirmfoto 2024-06-14 um 15 39 28

The number with decimals is shortened to "1 K", losing 23.457 % in precision. The number without decimals remains unchanged.

Analysis

The call shorten_number(1234.56789, "Germany", 5) is expected to return a rough representation of the number (1234.56789), using less than 5 digits. This can already be achieved by rounding to the nearest integer.

After

Bildschirmfoto 2024-06-14 um 15 39 05

The number with decimals is rounded to the nearest integer, which is less than five digits, losing only 0.035 % in precision.


This is an automatic backport of pull request #26760 done by Mergify.

Before, `shorten_number(1234.5678, "Germany", 5)` wrongly returned `"1 K"`. Now it correctly returns `"1235"`, which is less than five digits.

(cherry picked from commit 30756e2)
@barredterra barredterra merged commit 13715a0 into version-15-hotfix Jun 14, 2024
19 checks passed
@barredterra barredterra deleted the mergify/bp/version-15-hotfix/pr-26760 branch June 14, 2024 14:04
@frappe-pr-bot
Copy link
Collaborator

🎉 This PR is included in version 15.30.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants