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

feat: Do not panic when casting from an empty Series to pl.Decimal #14330

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

petrosbar
Copy link
Contributor

Closes #14002

More specifically, the PR attempts to address the following two issues:

1. Panic when casting from an empty Series without explicitly setting a scale:

pl.Series().cast(pl.Decimal)
# Panic: no scale!

In this case, we just set scale to 0, in the same way as it's done when constructing an empty Series with dtype=pl.Decimal:

pl.Series([], dtype=pl.Decimal) 

# shape: (0,)
# Series: '' [decimal[*,0]]
# [
# ] 

2. An error is raised when casting a Series, whose dtype is already pl.Decimal, without setting scale explicitly:

from decimal import Decimal

pl.Series([Decimal("10.0")]).cast(pl.Decimal)
# ComputeError: no scale!

I don't know why anyone would want to do that but raising an error seems too punishing; the existing scale can be kept. For example:

from decimal import Decimal

pl.Series([Decimal("10.0")]).cast(pl.Decimal)

# shape: (1,)
# Series: '' [decimal[*,1]]
# [
#         10
# ]

@github-actions github-actions bot added the fix Bug fix label Feb 7, 2024
@petrosbar petrosbar changed the title fix panic fix panic when casting empty Series to pl.Decimal without setting scale explicitly Feb 7, 2024
@petrosbar petrosbar changed the title fix panic when casting empty Series to pl.Decimal without setting scale explicitly fix(rust): Do not panic when casting from an empty Series to pl.Decimal Feb 7, 2024
@github-actions github-actions bot added the rust Related to Rust Polars label Feb 7, 2024
Copy link
Member

@ritchie46 ritchie46 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, seems good to me. Thanks.

@ritchie46 ritchie46 changed the title fix(rust): Do not panic when casting from an empty Series to pl.Decimal feat: Do not panic when casting from an empty Series to pl.Decimal Feb 7, 2024
@ritchie46 ritchie46 merged commit 2d12446 into pola-rs:main Feb 7, 2024
23 checks passed
@github-actions github-actions bot added enhancement New feature or an improvement of an existing feature python Related to Python Polars labels Feb 7, 2024
@petrosbar petrosbar deleted the fix/decimal-cast-panic branch February 7, 2024 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or an improvement of an existing feature fix Bug fix python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic when casting to Decimal
2 participants