-
Notifications
You must be signed in to change notification settings - Fork 22
Modifying to use rounding direction in RoundedDecimalField. #16
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
Conversation
1 similar comment
drf_braces/fields/custom.py
Outdated
| def to_internal_value(self, data): | ||
| return self.quantize(super(RoundedDecimalField, self).to_internal_value(data)) | ||
| def quantize(self, value): | ||
| if self.max_digits is None and isinstance(value, float): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is no longer a DecimalField if it will return float in some cases. it must return float in all cases unless its not required field and value is not provided
| ) | ||
|
|
||
| def to_internal_value(self, data): | ||
| return self.quantize(super(RoundedDecimalField, self).to_internal_value(data)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
older versions of DRF dont always call quantize on validation https://github.com/encode/django-rest-framework/blob/3.2.5/rest_framework/fields.py#L935
drf_braces/fields/custom.py
Outdated
| return data | ||
|
|
||
|
|
||
| class RoundedDecimalField(fields.DecimalField): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is duplicating the class definition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, I think this happened when I merged or something.
drf_braces/fields/custom.py
Outdated
| Currency field subclass of Decimal used for rounding currencies | ||
| to two decimal places. | ||
| """ | ||
| rounding = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not necessary as its redefined in `init
drf_braces/fields/custom.py
Outdated
| rounding = None | ||
|
|
||
| def __init__(self, max_digits=None, decimal_places=2, rounding=None, *args, **kwargs): | ||
| max_digits = max_digits or self.MAX_STRING_LENGTH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be safely removed as quantize does not add the context.prec when max digits is None. it was to prevent exceptions with older versions of DRF
| def test_init(self): | ||
| field = RoundedDecimalField() | ||
| self.assertIsNotNone(field.max_digits) | ||
| self.assertEqual(field.decimal_places, 2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add assert about rounding being set
This will now use rounding direction in RoundedDecimalField if specified.