django-socio-grpc icon indicating copy to clipboard operation
django-socio-grpc copied to clipboard

decimal.Decimal should have a string type as underlying type in proto files

Open AnyCPU opened this issue 6 months ago • 7 comments

i could be wrong, but it looks like a support of Decimal is implemented by using double type. it could cause troubles moving from original django grpc framework. it is a little bit tricky to use Decimal in the original, but possible, even without this google trick. i guess it would be nice to have a string type as underlying type in order to improve compatibility in general.

just fyi and maybe it could help someone else with the original:

  • server and client protos should use string;
  • serializer should put string;
  • a decimal value should be quantized in order to have room to fit into;
  • deserializer should use DecimalField;
  • a DecimalField should have max digits and decimal places enough for a value quantized;
  • otherwise you will get nothing or a lot of errors.

AnyCPU avatar May 21 '25 08:05 AnyCPU

Hello @AnyCPU

Thank you for your feedback. I understand your issue but having string to double cohercion can make a lot of issue specialy with financial data. The double type make this financial data exchange safe.

If you want to use string for double you can overide the fields in your serializer by creating a need serializer type class that handle that for you. By defaut a new type class is transformed as a string.

If you have a lot of fields you can create a generation plugin: https://django-socio-grpc.readthedocs.io/en/stable/features/proto-generation.html#proto-generation-plugins that everytime it see a double as ProtoField.type transform it to string. The example ReplaceAllTypeToStringGenerationPlugin give a good idea of how to do that. Then you can set it into your settings on DEFAULT_GENERATION_PLUGINS setting to apply it globally.

I do think that having decimal as double in proto file is a great improvement from the original and lot of user want it this way.

AMontagu avatar May 21 '25 12:05 AMontagu

@AMontagu no, i don't want to use string-for-double. i'm all about decimal, decimal is not double.

i see your point and i'm not IEEE's expert, but i would like leave following cites here:

Decimal numbers can be represented exactly.
In contrast, numbers like 1.1 and 2.2 do not have exact representations in binary floating point.
End users typically would not expect 1.1 + 2.2 to display
as 3.3000000000000003 as it does with binary floating point.

The exactness carries over into arithmetic.
In decimal floating point, 0.1 + 0.1 + 0.1 - 0.3 is exactly equal to zero.
In binary floating point, the result is 5.5511151231257827e-017.
While near to zero, the differences prevent reliable equality testing and differences can accumulate.
For this reason, decimal is preferred in accounting applications which have strict equality invariants.

source in Python: decimal — Decimal fixed-point and floating-point arithmetic

The type numeric can store numbers with a very large number of digits.
It is especially recommended for storing monetary amounts
and other quantities where exactness is required.

source in Postgre: Arbitrary Precision Numbers

Thanks.

AnyCPU avatar May 21 '25 13:05 AnyCPU

@AnyCPU Indeed, See your point. Looking into it and see that there is no native implementation of Decimal in protobuf before protobuf 4. Look like there is a proto file for that https://github.com/googleapis/googleapis/blob/master/google/type/decimal.proto

Will need to look more into it and will let the issue open. But, I am really busy lately so it may take long time to improve. Also it may be interesting to not implement it myself and wait for it to be implement in protobuf and grpc directly.

AMontagu avatar May 21 '25 14:05 AMontagu

@AMontagu you can also find this link https://github.com/googleapis/googleapis/blob/master/google/type/decimal.proto in a first message here. to be honest it adds nothing special, you can define your own definition of decimal or just use a string in a language specific library in order to support decimal.

how and when it will be implemented officially? i would say never because of https://github.com/protocolbuffers/protobuf/issues/4406

this issue keeps me using the original grpc framework because it allows to use strings as intermediate data format and to use decimals "as is" in python, at the moment it is win-win.

AnyCPU avatar May 21 '25 15:05 AnyCPU

@AnyCPU why not patching https://github.com/socotecio/django-socio-grpc/blob/master/django_socio_grpc/protobuf/proto_classes.py#L859 and https://github.com/socotecio/django-socio-grpc/blob/master/django_socio_grpc/protobuf/proto_classes.py#L819 to make them string ? (if you go with the patching in need to be done as first in the ROOT_HANDLERS_HOOK method

Or as in my first comment say, use a generation plugin ?

AMontagu avatar May 21 '25 15:05 AMontagu

@AMontagu long story short: i have extremely limited time and no additional resources to contribute at the moment.

AnyCPU avatar May 21 '25 16:05 AnyCPU

@AnyCPU it's not contributing it's patching inside your code to make it what you want. See it like integration time when using a tool.

But I understand that if you have something working now and not much time, this integration time mais be the reason to not use the librairy.

AMontagu avatar May 22 '25 08:05 AMontagu