Skip to content

Commit

Permalink
feat: configurable rounding methods (v14 port) (#20330)
Browse files Browse the repository at this point in the history
* feat: configurable rounding methods

* feat: Allow specifying rounding method in `flt`

* feat: implement custom rounding in JS

* fix(UX): Warn about changing rounding method

* refactor: split rounding methods in functions

* refactor: change rounding method names (#20299)

These are easy to understand.

Added third method for corrected banker's rounding.

* chore: typo

* fix: corrected banker's rounding

closes #19570

* fix: Make corrected bankers rounding default method

* fix: skip 0 for rounding

* Revert "fix: Make corrected bankers rounding default method"

This reverts commit 709edf1.

* fix: add patch for setting current method as default
  • Loading branch information
ankush committed Mar 14, 2023
1 parent 2a88313 commit 0d60385
Show file tree
Hide file tree
Showing 11 changed files with 432 additions and 23 deletions.
104 changes: 104 additions & 0 deletions cypress/integration/rounding.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
context("Rounding behaviour", () => {
before(() => {
cy.login();
cy.visit("/app/");
});

it("Commercial Rounding", () => {
cy.window()
.its("flt")
.then((flt) => {
let rounding_method = "Commercial Rounding";

expect(flt("0.5", 0, null, rounding_method)).eq(1);
expect(flt("0.3", null, null, rounding_method)).eq(0.3);

expect(flt("1.5", 0, null, rounding_method)).eq(2);

// positive rounding to integers
expect(flt(0.4, 0, null, rounding_method)).eq(0);
expect(flt(0.5, 0, null, rounding_method)).eq(1);
expect(flt(1.455, 0, null, rounding_method)).eq(1);
expect(flt(1.5, 0, null, rounding_method)).eq(2);

// negative rounding to integers
expect(flt(-0.5, 0, null, rounding_method)).eq(-1);
expect(flt(-1.5, 0, null, rounding_method)).eq(-2);

// negative precision i.e. round to nearest 10th
expect(flt(123, -1, null, rounding_method)).eq(120);
expect(flt(125, -1, null, rounding_method)).eq(130);
expect(flt(134.45, -1, null, rounding_method)).eq(130);
expect(flt(135, -1, null, rounding_method)).eq(140);

// positive multiple digit rounding
expect(flt(1.25, 1, null, rounding_method)).eq(1.3);
expect(flt(0.15, 1, null, rounding_method)).eq(0.2);
expect(flt(2.675, 2, null, rounding_method)).eq(2.68);

// negative multiple digit rounding
expect(flt(-1.25, 1, null, rounding_method)).eq(-1.3);
expect(flt(-0.15, 1, null, rounding_method)).eq(-0.2);
});
});

it("Banker's Rounding", () => {
cy.window()
.its("flt")
.then((flt) => {
let rounding_method = "Banker's Rounding";

expect(flt("0.5", 0, null, rounding_method)).eq(0);
expect(flt("0.3", null, rounding_method)).eq(0.3);

expect(flt("1.5", 0, null, rounding_method)).eq(2);

// positive rounding to integers
expect(flt(0.4, 0, null, rounding_method)).eq(0);
expect(flt(0.5, 0, null, rounding_method)).eq(0);
expect(flt(1.455, 0, null, rounding_method)).eq(1);
expect(flt(1.5, 0, null, rounding_method)).eq(2);

// negative rounding to integers
expect(flt(-0.5, 0, null, rounding_method)).eq(0);
expect(flt(-1.5, 0, null, rounding_method)).eq(-2);

// negative precision i.e. round to nearest 10th
expect(flt(123, -1, null, rounding_method)).eq(120);
expect(flt(125, -1, null, rounding_method)).eq(120);
expect(flt(134.45, -1, null, rounding_method)).eq(130);
expect(flt(135, -1, null, rounding_method)).eq(140);

// positive multiple digit rounding
expect(flt(1.25, 1, null, rounding_method)).eq(1.2);
expect(flt(0.15, 1, null, rounding_method)).eq(0.2);
expect(flt(2.675, 2, null, rounding_method)).eq(2.68);
expect(flt(-2.675, 2, null, rounding_method)).eq(-2.68);

// negative multiple digit rounding
expect(flt(-1.25, 1, null, rounding_method)).eq(-1.2);
expect(flt(-0.15, 1, null, rounding_method)).eq(-0.2);

// Nearest number and not even (the default behaviour)
expect(flt(0.5, 0, null, rounding_method)).eq(0);
expect(flt(1.5, 0, null, rounding_method)).eq(2);
expect(flt(2.5, 0, null, rounding_method)).eq(2);
expect(flt(3.5, 0, null, rounding_method)).eq(4);

expect(flt(0.05, 1, null, rounding_method)).eq(0.0);
expect(flt(1.15, 1, null, rounding_method)).eq(1.2);
expect(flt(2.25, 1, null, rounding_method)).eq(2.2);
expect(flt(3.35, 1, null, rounding_method)).eq(3.4);

expect(flt(-0.5, 0, null, rounding_method)).eq(0);
expect(flt(-1.5, 0, null, rounding_method)).eq(-2);
expect(flt(-2.5, 0, null, rounding_method)).eq(-2);
expect(flt(-3.5, 0, null, rounding_method)).eq(-4);

expect(flt(-0.05, 1, null, rounding_method)).eq(0.0);
expect(flt(-1.15, 1, null, rounding_method)).eq(-1.2);
expect(flt(-2.25, 1, null, rounding_method)).eq(-2.2);
expect(flt(-3.35, 1, null, rounding_method)).eq(-3.4);
});
});
});
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import frappe
from frappe.database.database import savepoint


def execute():
"""set default rounding method"""

with savepoint(Exception):
settings = frappe.get_doc("System Settings")
settings.rounding_method = "Banker's Rounding (legacy)"
settings.flag.ignore_mandatory = True
settings.save(ignore_version=True)
17 changes: 17 additions & 0 deletions frappe/core/doctype/system_settings/system_settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,21 @@ frappe.ui.form.on("System Settings", {
first_day_of_the_week(frm) {
frm.re_setup_moment = true;
},

rounding_method: function (frm) {
if (frm.doc.rounding_method == frappe.boot.sysdefaults.rounding_method) return;
let msg = __(
"Changing rounding method on site with data can result in unexpected behaviour."
);
msg += "<br>";
msg += __("Do you still want to proceed?");

frappe.confirm(
msg,
() => {},
() => {
frm.set_value("rounding_method", frappe.boot.sysdefaults.rounding_method);
}
);
},
});
14 changes: 11 additions & 3 deletions frappe/core/doctype/system_settings/system_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@
"date_format",
"time_format",
"number_format",
"first_day_of_the_week",
"column_break_7",
"float_precision",
"currency_precision",
"first_day_of_the_week",
"rounding_method",
"sec_backup_limit",
"backup_limit",
"encrypt_backup",
Expand Down Expand Up @@ -540,12 +541,19 @@
"fieldname": "apply_perm_level_on_api_calls",
"fieldtype": "Check",
"label": "Apply Perm Level on API calls (Recommended)"
},
{
"default": "Banker's Rounding (legacy)",
"fieldname": "rounding_method",
"fieldtype": "Select",
"label": "Rounding Method",
"options": "Banker's Rounding (legacy)\nBanker's Rounding\nCommercial Rounding"
}
],
"icon": "fa fa-cog",
"issingle": 1,
"links": [],
"modified": "2023-03-03 11:47:24.867356",
"modified": "2023-03-10 12:23:45.248125",
"modified_by": "Administrator",
"module": "Core",
"name": "System Settings",
Expand All @@ -564,4 +572,4 @@
"sort_order": "ASC",
"states": [],
"track_changes": 1
}
}
4 changes: 4 additions & 0 deletions frappe/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,10 @@ class ExecutableNotFound(FileNotFoundError):
pass


class InvalidRoundingMethod(FileNotFoundError):
pass


class InvalidRemoteException(Exception):
pass

Expand Down
1 change: 1 addition & 0 deletions frappe/patches.txt
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,4 @@ frappe.patches.v14_0.update_multistep_webforms
frappe.patches.v14_0.drop_unused_indexes
frappe.patches.v14_0.disable_email_accounts_with_oauth
frappe.patches.v14_0.remove_manage_subscriptions_from_navbar
frappe.core.doctype.system_settings.patches.set_default_rounding_method
68 changes: 56 additions & 12 deletions frappe/public/js/frappe/utils/number_format.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import "./datatype";

if (!window.frappe) window.frappe = {};

function flt(v, decimals, number_format) {
function flt(v, decimals, number_format, rounding_method) {
if (v == null || v == "") return 0;

if (!(typeof v === "number" || String(parseFloat(v)) == v)) {
Expand All @@ -30,7 +30,7 @@ function flt(v, decimals, number_format) {
}

v = parseFloat(v);
if (decimals != null) return _round(v, decimals);
if (decimals != null) return _round(v, decimals, rounding_method);
return v;
}

Expand Down Expand Up @@ -173,16 +173,60 @@ function get_number_format_info(format) {
return info;
}

function _round(num, precision) {
var is_negative = num < 0 ? true : false;
var d = cint(precision);
var m = Math.pow(10, d);
var n = +(d ? Math.abs(num) * m : Math.abs(num)).toFixed(8); // Avoid rounding errors
var i = Math.floor(n),
f = n - i;
var r = !precision && f == 0.5 ? (i % 2 == 0 ? i : i + 1) : Math.round(n);
r = d ? r / m : r;
return is_negative ? -r : r;
function _round(num, precision, rounding_method) {
rounding_method =
rounding_method || frappe.boot.sysdefaults.rounding_method || "Banker's Rounding (legacy)";

let is_negative = num < 0 ? true : false;

if (rounding_method == "Banker's Rounding (legacy)") {
var d = cint(precision);
var m = Math.pow(10, d);
var n = +(d ? Math.abs(num) * m : Math.abs(num)).toFixed(8); // Avoid rounding errors
var i = Math.floor(n),
f = n - i;
var r = !precision && f == 0.5 ? (i % 2 == 0 ? i : i + 1) : Math.round(n);
r = d ? r / m : r;
return is_negative ? -r : r;
} else if (rounding_method == "Banker's Rounding") {
if (num == 0) return 0.0;
precision = cint(precision);

let multiplier = Math.pow(10, precision);
num = Math.abs(num) * multiplier;

let floor_num = Math.floor(num);
let decimal_part = num - floor_num;

// For explanation of this method read python flt implementation notes.
let epsilon = 2.0 ** (Math.log2(Math.abs(num)) - 52.0);

if (Math.abs(decimal_part - 0.5) < epsilon) {
num = floor_num % 2 == 0 ? floor_num : floor_num + 1;
} else {
num = Math.round(num);
}
num = num / multiplier;
return is_negative ? -num : num;
} else if (rounding_method == "Commercial Rounding") {
if (num == 0) return 0.0;

let digits = cint(precision);
let multiplier = Math.pow(10, digits);

num = num * multiplier;

// For explanation of this method read python flt implementation notes.
let epsilon = 2.0 ** (Math.log2(Math.abs(num)) - 52.0);
if (is_negative) {
epsilon = -1 * epsilon;
}

num = Math.round(num + epsilon);
return num / multiplier;
} else {
throw new Error(`Unknown rounding method ${rounding_method}`);
}
}

function roundNumber(num, precision) {
Expand Down
Loading

0 comments on commit 0d60385

Please sign in to comment.