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

[Feature request] Add support for Decimal type #867

Open
Munksgaard opened this issue Feb 24, 2024 · 12 comments
Open

[Feature request] Add support for Decimal type #867

Munksgaard opened this issue Feb 24, 2024 · 12 comments

Comments

@Munksgaard
Copy link

Munksgaard commented Feb 24, 2024

As per this comment I am opening this issue.

I am working with monetary data, for which floats are largely discouraged. As a result, I am using the fairly popular Decimal package and the associated Ecto support to store monetary values in my database and process them in my app.

I’m also a big fan of Livebook and Explorer to process my data. However, Explorer does not currently have support for Decimals, which means that I have to manually convert to floats, losing all the benefits of working with Decimal in the first place.

Therefore, I am wondering what it would take to add support for Decimals to Explorer and humbly requesting it for consideration.

@Munksgaard Munksgaard changed the title [ [Feature request] Add support for Decimal type Feb 24, 2024
@billylanchantin
Copy link
Contributor

(Carrying over some of my comment as well)

Polars does support a decimal datatype:

If we want to support it as well, the tricky part will be representing things in Elixir. I see two options:

I haven't played with the Decimal datatype in Polars or Arrow, so I don't know if it maps nicely to the Decimal library's representation. But I have no objections to exploring either option (or some other thing I didn't think of).

@josevalim
Copy link
Member

There is a reference here: https://arrow.apache.org/docs/r/reference/data-type.html

The implementation is straight-forward, it is stored as a large integer, which we can easily convert to Decimal using Decimal.new/3. If a list of Decimals is given as input, we will need to compute the scale, using Decimal.scale. And for precision, we can use the maximum one.

@billylanchantin
Copy link
Contributor

👍 I think I'm comfortable adding Decimal as a dep.

@Munksgaard PRs welcome! If you're not able to, then I can take a crack when I have some time.

@Munksgaard
Copy link
Author

Munksgaard commented Feb 24, 2024

One complication that I can see is that Decimal allows both NaN and Infinity as values, which we don't have representations for in Polars as far as I can tell.

@josevalim
Copy link
Member

Yes, we should raise when encoding those. We will already have to do a prepass converting all decimals into int::128 or int::256, so we can raise on NaN and Infinity.

@Munksgaard
Copy link
Author

@Munksgaard PRs welcome! If you're not able to, then I can take a crack when I have some time.

I would love to help out, but realistically I won't be able to contribute anything significant in the next couple of weeks :-(

@cigrainger
Copy link
Member

I'm comfortable with Decimal as a dep as well. It doesn't carry its own deps, pretty lightweight.

@billylanchantin
Copy link
Contributor

Rustler now supports 128 bit integers (thanks for initiating that, @Munksgaard!):

I think we can consider this issue blocked until rustler does another release. Otherwise we'd have to go off their master branch, which probably isn't worth the headache since they seem to release regularly.

@LostKobrakai
Copy link

Seems like this has already been released with rustler 0.32.1

@philss
Copy link
Member

philss commented Mar 21, 2024

The update of Rustler to v0.32.1 has landed on main :)

@alexpearce
Copy link

I had a crack at adding Decimal support and managed to add reading without too much trouble: alexpearce@709aa67

I found adding write support more challenging. It seems that Decimal support in polars-core isn't quite as mature as for other dtypes so write support requires a bit more work.

@billylanchantin
Copy link
Contributor

@alexpearce Feel free to open a PR. If you're still having trouble, we can discuss there why write support is more difficult.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants