Convert DecimalField to String rather than Float

[gsvr]

DecimalField entries are currently being converted to Float (

graphene-django/graphene_django/form_converter.py

Lines 56 to 59 in 8f5a425

	@convert_form_field.register(forms.DecimalField)
	@convert_form_field.register(forms.FloatField)
	def convert_form_field_to_float(field):
	return Float(description=field.help_text, required=field.required)

)

Full support for Decimal would be great, but in the meantime I propose that DecimalField should be converted to String instead. Once you've converted a Decimal to a Float, you've probably lost the benefit of why you used Decimal in the first place, since you'll now have values like 3.14000000000000012, rather than 3.14. If it is converted to String, it is simple enough for the user to convert that String back to Decimal, whereas converting from Float to Decimal is not straightforward.

[picturedots]

As a workaround for this particular problem, I created a Decimal type, and then am explicitly setting that field in the DjangoObjectType class. For example, I created a Decimal type like this

import decimal

from graphene.types import Scalar
from graphql.language import ast

class Decimal(Scalar):
    """ 
    The `Decimal` scalar type represents a python Decimal.
    """
    @staticmethod
    def serialize(dec):
        assert isinstance(dec, decimal.Decimal), (
            'Received not compatible Decimal "{}"'.format(repr(dec))
        )
        return str(dec)
    
    @staticmethod
    def parse_value(value):
        return decimal.Decimal(value)
    
    @classmethod
    def parse_literal(cls, node):
        if isinstance(node, ast.StringValue):
            return cls.parse_value(node.value)

and then explicitly replace the field like this:

class MyModelType(DjangoObjectType):
    my_decimal_field = Decimal()
    class Meta:
        model = MyModel

assuming that I have a Django model MyModel with Decimal field my_decimal_field.

[zbyte64]

Decimal type landed in graphene: graphql-python/graphene#703

Edit: Appears it isn't released yet but is merged, need graphene>2.1.3

[picturedots]

Yeah, I finally got around to adding my Decimal type into graphene. It should be possible to fix graphene-django's implementation of DecimalField after the next release of graphene to actually use a Decimal, and it won't be necessary to convert to a String (as per the title of this issue). Note that the JSON output is implemented as string though.

[gsvr]

Brilliant, thank you @picturedots!

[picturedots]

@gsvr I didn't make the graphene-django change that you noted in form_converter.py above, so not sure if this is issue should be closed. Without that change, it looks like Decimals are still explicitly converted to floats.

[clarkmoody]

Waiting on this feature as well.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ulgens]

@gsvr @jkimbo Is this still relevant?

[ulgens]

The issue is still relevant. We talked with @jkimbo about how to fix it. My work will be on #958 and i hope to complete in next couple days.

[gsvr]

Thanks @ulgens
I agree that it's still relevant - I've used a workaround in the meantime but this would be a good general improvement imho

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[gsvr]

Thanks to everyone who contributed to resolving this! 👏 👏

[i6mi6]

Has this been documented in the changelog? It broke our existing app logic because now suddenly we started getting strings rather than floats. Is there a way to default to previous behavior?

[beaugunderson]

Huge +1 for adding a configuration setting for changing the default behavior here; it looks like this change happened in a minor release? It should have been a major release as it's a breaking change (it broke our codebase as well).

[i6mi6]

@beaugunderson I ended up forking the repo then modifying it with this commit
i6mi6@d95b19d
In my case the branch name is v2.15.0_decimal then I added it to requirements.txt as follows:

...
graphene==2.1.8
git+git://github.com/i6mi6/graphene-django.git@v2.15.0_decimal#egg=graphene-django
...

[chidimo]

As a workaround for this particular problem, I created a Decimal type, and then am explicitly setting that field in the DjangoObjectType class. For example, I created a Decimal type like this

import decimal

from graphene.types import Scalar
from graphql.language import ast

class Decimal(Scalar):
    """ 
    The `Decimal` scalar type represents a python Decimal.
    """
    @staticmethod
    def serialize(dec):
        assert isinstance(dec, decimal.Decimal), (
            'Received not compatible Decimal "{}"'.format(repr(dec))
        )
        return str(dec)
    
    @staticmethod
    def parse_value(value):
        return decimal.Decimal(value)
    
    @classmethod
    def parse_literal(cls, node):
        if isinstance(node, ast.StringValue):
            return cls.parse_value(node.value)

and then explicitly replace the field like this:

class MyModelType(DjangoObjectType):
    my_decimal_field = Decimal()
    class Meta:
        model = MyModel

assuming that I have a Django model MyModel with Decimal field my_decimal_field.

This approach is very helpful.
Thank you