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: rounding of percentage fields #36167

Merged
merged 1 commit into from
Jul 30, 2023

Conversation

barredterra
Copy link
Collaborator

@barredterra barredterra commented Jul 17, 2023

Always round with precision of 2

  • per_delivered
  • per_picked
  • per_billed
  • per_received

These are of type "Percentage", which usually is a value between 0 and 100. Something that is delivered/picked/billed/received > 99.99% should probably be marked as done.

In the past we used a precision of 6 in some places. Orders would have to be very large for 0.004 % to be valuable enough for the order to to stay open.

Always round with precision of 2
@barredterra barredterra force-pushed the percentage-rounding branch from 79315ca to ecaf0ab Compare July 17, 2023 14:45
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #36167 (ecaf0ab) into develop (3b246fd) will not change coverage.
The diff coverage is 0.00%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #36167   +/-   ##
========================================
  Coverage    64.08%   64.08%           
========================================
  Files          783      783           
  Lines        60668    60668           
========================================
  Hits         38879    38879           
  Misses       21789    21789           
Impacted Files Coverage Δ
erpnext/controllers/website_list_for_contact.py 56.02% <0.00%> (ø)

@rohitwaghchaure
Copy link
Collaborator

@barredterra Using the precision as 2 will also cause the issue, some clients want 6 and some 2. I guess we need a configuration to set this value.

@barredterra
Copy link
Collaborator Author

They really wish for 0.4 parts in a million to keep their orders open? Should we just use the precision setting of the individual fields then?

@barredterra barredterra self-assigned this Jul 25, 2023
@ankush
Copy link
Member

ankush commented Jul 25, 2023

@barredterra can you share an example where it doesn't tip over to 100 when it's very near 100?

Perhaps sparing use of Decimal type here can solve the problem. https://docs.python.org/3/library/decimal.html

@barredterra
Copy link
Collaborator Author

For example, after three payments/deliveries/receipts of 33.333% a transaction would still be unresolved because flt(33.333 * 3) != 100 while flt(33.333 * 3, 2) == 100

@deepeshgarg007 deepeshgarg007 merged commit c6b024c into frappe:develop Jul 30, 2023
@barredterra barredterra deleted the percentage-rounding branch July 30, 2023 09:56
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants