Fix zero-amount transfers and other rounding issues #164

zorun · 2017-01-02T14:05:57Z

These are workarounds for rounding issues.

aavenel · 2017-01-02T14:12:33Z

budget/models.py

@@ -61,9 +61,9 @@ def get_transactions_to_settle_bill(self):
        credits, debts, transactions = [],[],[]
        # Create lists of credits and debts
        for person in self.members:
-            if balance[person.id] > 0:
+            if round(balance[person.id], 2) > 0:


I think it'll be better to do the rounding in the balance() function instead.

This is what I was also thinking, but I decided against it since balance() seems to be used in a lot of other places. Some calling functions may need the full floating-point precision to work.

It's a bit like unicode vs. bytestrings: always use unicode internally, and only decode/encode to UTF-8 bytestrings when handling inputs/ouputs.

Ok, looks good to me.

JocelynDelalande · 2017-01-02T19:48:20Z

Thanks a lot, welcome here :-)

👍 (tested & reviewed), although I don't know how to trigger the problematic cases, so my test is barely "it does not breaks the existing".

Should be easy to provide a test, if you have time, at least for 72f177a ?

This workarounds a rounding issue caused by the (incorrect) usage of floats for bill amounts. This fixes spiral-project#138

A user with a "0.00" balance would have either a "+0.00" in green or a "-0.00" in red, depending on the exact value of the floating-point value. Fix this by simply rounding to 2 digits before comparing to zero.

zorun · 2017-01-03T20:39:24Z

@JocelynDelalande good idea :)

I have added a test for the zero-transfer issue, this is the output before applying b507a5a :

======================================================================
FAIL: test_settle_zero (__main__.BudgetTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests.py", line 622, in test_settle_zero
    msg='%f is equal to zero after rounding' % t['amount'])
AssertionError: 0.003333 is equal to zero after rounding

almet · 2017-01-16T20:14:04Z

Thanks :)

Fix zero-amount transfers and other rounding issues

aavenel reviewed Jan 2, 2017

View reviewed changes

zorun mentioned this pull request Jan 2, 2017

In the settle page remove exchanges of values: 0 #138

Closed

Baptiste Jonglez added 3 commits January 3, 2017 21:37

Add a test to catch zero-amount transfers in the settle page

543df7c

Prevent transfers with a zero amount in the settle page

b507a5a

This workarounds a rounding issue caused by the (incorrect) usage of floats for bill amounts. This fixes spiral-project#138

Workaround rounding issues when displaying balance of users

d6cf890

A user with a "0.00" balance would have either a "+0.00" in green or a "-0.00" in red, depending on the exact value of the floating-point value. Fix this by simply rounding to 2 digits before comparing to zero.

zorun force-pushed the fix_zero_transfers branch from 3c4c6e4 to d6cf890 Compare January 3, 2017 20:37

almet merged commit dc75a72 into spiral-project:master Jan 16, 2017

Jojo144 pushed a commit to Jojo144/ihatemoney that referenced this pull request Mar 21, 2020

Merge pull request spiral-project#164 from zorun/fix_zero_transfers

db95fcd

Fix zero-amount transfers and other rounding issues

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix zero-amount transfers and other rounding issues #164

Fix zero-amount transfers and other rounding issues #164

zorun commented Jan 2, 2017

aavenel Jan 2, 2017

zorun Jan 2, 2017

aavenel Jan 2, 2017

JocelynDelalande commented Jan 2, 2017

zorun commented Jan 3, 2017

almet commented Jan 16, 2017

Fix zero-amount transfers and other rounding issues #164

Fix zero-amount transfers and other rounding issues #164

Conversation

zorun commented Jan 2, 2017

aavenel Jan 2, 2017

Choose a reason for hiding this comment

zorun Jan 2, 2017

Choose a reason for hiding this comment

aavenel Jan 2, 2017

Choose a reason for hiding this comment

JocelynDelalande commented Jan 2, 2017

zorun commented Jan 3, 2017

almet commented Jan 16, 2017