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

New rule: Decimal with float argument #11840

Closed
kbaskett248 opened this issue Jun 11, 2024 · 6 comments
Closed

New rule: Decimal with float argument #11840

kbaskett248 opened this issue Jun 11, 2024 · 6 comments
Labels
rule Implementing or modifying a lint rule

Comments

@kbaskett248
Copy link
Contributor

I'm interested in adding a new rule to check for Decimal being called with a float literal argument. For example:

# Don't
d = Decimal(1.234)
# Do
d = Decimal('1.234')

This seems like a generally useful check. I searched for open issues and PRs involving Decimal and didn't find any mention of this type of check. I'm happy to work on a PR, but wanted to make sure this check makes sense in ruff. Please let me know if I'm missing something or there's any reason this check wouldn't be accepted.

Thanks!

@charliermarsh
Copy link
Member

I don't think we have a check for this. Can you say more on why it's something to be avoided?

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Jun 12, 2024
@mayanyax
Copy link

Consider:

>>> Decimal(0.3) == Decimal(0.1) + Decimal(0.1) + Decimal(0.1)
False
>>> Decimal('0.3') == Decimal('0.1') + Decimal('0.1') + Decimal('0.1')
True

In the first example you basically creating an exact representation of the floating point approximation to 0.1. To my mind that is almost certainly not what is wanted as it seems to defeat the purpose of using Decimal in the first place.

@kbaskett248
Copy link
Contributor Author

Yeah, @mayanyax is exactly right. Passing a float literal could introduce imprecision, which is exactly what we're hoping to avoid.

@mjgp2
Copy link

mjgp2 commented Jul 18, 2024

This would be a good addition, I have use cases where this has caused potential bugs.

@Matthieu-LAURENT39
Copy link
Contributor

this is a footgun i keep forgetting, and i would definitely enjoy seeing this rule implemented.

@AlexWaygood
Copy link
Member

Fixed by #12909

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

6 participants