Fix warnings for Decimal >= 1.6.0 #643
Conversation
|
Thanks! |
| case Decimal.parse(value) do | ||
| {:ok, decimal} -> {:ok, decimal} | ||
| _ -> :error | ||
| end | ||
| end | ||
|
|
||
| def parse(%Absinthe.Blueprint.Input.Float{value: value}) do | ||
| decimal = Decimal.new(value) | ||
| def parse(%Absinthe.Blueprint.Input.Float{value: value}) when is_float(value) do |
There was a problem hiding this comment.
If we add the is_float guard then we can remove the:
if Decimal.nan?(decimal), do: :error, else: {:ok, decimal}line below, because for a float argument we're guaranteed to get a decimal that is not a nan. Same goes for an integer argument below. Was the intent before that value in Float{value: value} could have been a "NaN"?
I think this check makes sense for the clause above: String{value: value}) when is_binary(value).
Btw, in Ecto we're preventing NaN and +Inf/-Inf decimals as support for these varies between databases: https://github.com/elixir-ecto/ecto/blob/v3.0.3/lib/ecto/type.ex#L1198.
There was a problem hiding this comment.
From my reading of the spec NaN should be treated as null. I'll adjust accordingly.
| def parse(%Absinthe.Blueprint.Input.Float{value: value}) do | ||
| decimal = Decimal.new(value) | ||
| def parse(%Absinthe.Blueprint.Input.Float{value: value}) when is_float(value) do | ||
| decimal = Decimal.from_float(value) |
There was a problem hiding this comment.
for this we need to bump decimal requirement from ~> 1.0 to at least ~> 1.5 as this function was only introduced in v1.5.0.
There was a problem hiding this comment.
Hi, what happened to this fix? We still get loads of warning for Decimal 1.6.0 when combined with newest Absinthe. Would be great to upgrade finally. Thanks.
Closes #642