Port to Python 3 and Django 3#9
Open
ysangkok wants to merge 2 commits into
Open
Conversation
With three people sharing an expense that costs 1, person one paying 0.34, and the others paying 0.33, this commit makes it such that the result matrix is empty with the basic calc method.
Collaborator
Author
|
@tykling seems like the rounding issue provoked by having three people share an expense that costs 1 is only appearing with the basic calc method. Is it important to fix it or can the basic calc just be removed? I think maybe you commented once that you thought it was optimizing too much, so that you wouldn't understand why you'd have to give money to somebody? Anyway, I added a commit that fixes the issue for the situation previously described, I don't know if there are other test cases. The fix is kinda of a hack, simply modifying the algorithm so that it treats 0.01 as a zero. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I did some casual testing and everything I tried works.
Things to watch out for:
on_delete is now mandatory, and I set CASCADE everywhere, because I assumed the app will check before trying to delete something that is depended on by a ForeignKey.
getcurrency was rewritten to use the European Central Bank, since the old URL at the Danish Central Bank (not the "national bank" in English :P) is not working.