[ruff] Implement check for Decimal called with a float literal (RUF032) by kbaskett248 · Pull Request #12909 · astral-sh/ruff

kbaskett248 · 2024-08-15T18:50:55Z

Summary

This PR adds a new check for a Decimal call with a float argument. As described in #11840, this is a problem when precision is important, which is usually why Decimal is used in the first place. To pull an example by @mayanyax from the issue:

>>> 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

The check supports positive and negative floats. It also includes a fix that wraps the float in quotes.

Test Plan

Test cases were added for the new rule.

github-actions · 2024-08-15T19:04:46Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+25 -0 violations, +0 -0 fixes in 4 projects; 50 projects unchanged)

RasaHQ/rasa (+14 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ tests/core/test_utils.py:41:29: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:49:28: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:49:52: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:49:77: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:55:52: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:55:76: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:56:40: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:71:18: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:77:19: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:77:33: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:77:48: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:80:43: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:80:57: RUF032 `Decimal()` called with float literal argument
+ tests/core/test_utils.py:80:82: RUF032 `Decimal()` called with float literal argument

apache/airflow (+3 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ tests/providers/amazon/aws/transfers/test_dynamodb_to_s3.py:94:21: RUF032 `Decimal()` called with float literal argument
+ tests/providers/google/cloud/transfers/test_cassandra_to_gcs.py:132:27: RUF032 `Decimal()` called with float literal argument
+ tests/www/views/test_views_trigger_dag.py:104:28: RUF032 `Decimal()` called with float literal argument

ibis-project/ibis (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ ibis/expr/datatypes/tests/test_value.py:48:26: RUF032 `Decimal()` called with float literal argument

pandas-dev/pandas (+7 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ pandas/tests/dtypes/cast/test_downcast.py:36:39: RUF032 `Decimal()` called with float literal argument
+ pandas/tests/dtypes/cast/test_downcast.py:38:39: RUF032 `Decimal()` called with float literal argument
+ pandas/tests/dtypes/test_missing.py:326:21: RUF032 `Decimal()` called with float literal argument
+ pandas/tests/tools/test_to_numeric.py:195:40: RUF032 `Decimal()` called with float literal argument
+ pandas/tests/tools/test_to_numeric.py:210:31: RUF032 `Decimal()` called with float literal argument
+ pandas/tests/tools/test_to_numeric.py:210:60: RUF032 `Decimal()` called with float literal argument
+ pandas/tests/tools/test_to_numeric.py:213:37: RUF032 `Decimal()` called with float literal argument

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
RUF032	25	25	0	0	0

MichaReiser

Nice. I'd love to get some input from @AlexWaygood but this looks great

MichaReiser · 2024-08-16T06:35:58Z

+
+fn fix_float_literal(range: TextRange, float_literal: &str) -> Fix {
+    let content = format!("\"{float_literal}\"");
+    Fix::unsafe_edit(Edit::range_replacement(content, range))


What's your reasoning for making it an unsafe edit. Is it because it changes the precision of the values and, therefore, could result in runtime changes?

I think an unsafe edit is appropriate here... in most cases, you'll be changing the underlying value of the Decimal object being constructed. There might be code elsewhere that implicitly depended on the previous, "wrong" value that was caused by constructing the Decimal instance from the float, so it's entirely possible that fixing this error could unexpectedly cause your code to break.

@kbaskett248, it would be great if you could add one or two sentences to the docs explaining why this is an unsafe fix, like how we do with other rules.

Honestly it was mostly unfamiliarity with the apis, but @AlexWaygood brings up a good point.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

codspeed-hq · 2024-08-16T18:54:45Z

CodSpeed Performance Report

Merging #12909 will not alter performance

_{Comparing kbaskett248:rule/float-literal-decimal (9a8fd54) with main (65de8f2)}

Summary

✅ 32 untouched benchmarks

AlexWaygood · 2024-08-16T18:56:28Z

Merging #12909 will degrade performances by 5.27%

You can ignore that, it's been super flaky recently

…cimal

kbaskett248 · 2024-08-16T19:18:08Z

Thanks for your help with this @AlexWaygood and @MichaReiser! Any other updates you'd like me to make?

AlexWaygood · 2024-08-18T16:16:43Z

+fn fix_float_literal(range: TextRange, float_literal: &str) -> Fix {
+    let content = format!("\"{float_literal}\"");
+    Fix::unsafe_edit(Edit::range_replacement(content, range))


One last thing. Here we should use the quote style that the user is generally using for the rest of their file, which won't necessarily be double quotes if they're not using autoformatting. We can do this with the checker.stylist():

Suggested change

fn fix_float_literal(range: TextRange, float_literal: &str) -> Fix {

let content = format!("\"{float_literal}\"");

Fix::unsafe_edit(Edit::range_replacement(content, range))

fn fix_float_literal(range: TextRange, float_literal: &str, stylist: &Stylist) -> Fix {

let quote = stylist.quote();

let content = format!("{quote}{float_literal}{quote}");

Fix::unsafe_edit(Edit::range_replacement(content, range))

You'll just want to pass checker.stylist() into this function at the callsite, and you'll want to add use ruff_python_codegen::Stylist; to the top of the file so that you can use it here in the type signature

kbaskett248 · 2024-08-19T16:16:21Z

Thank you both for your help getting this in!

AlexWaygood · 2024-08-19T16:17:19Z

Thank you both for your help getting this in!

Thanks for the great contribution!

kbaskett248 added 9 commits August 15, 2024 14:37

Define new rule

ad933c2

Add float_literal_decimal to ruff module

65a7387

Add error codes

ea12aaf

Call new lint rule

52b61fa

Add test fixture for rule

44d86e2

Add test case

276704c

Add test snapshot

f4fe59f

Update schema

4f29a8f

Fix clippy errors

6dff975

MichaReiser added rule Implementing or modifying a lint rule preview Related to preview mode features labels Aug 16, 2024

MichaReiser approved these changes Aug 16, 2024

View reviewed changes

kbaskett248 added 4 commits August 16, 2024 13:55

Rename rule to FloatLiteralDecimal -> DecimalFromFloatLiteral

d67c1ab

Update docs for rule

b6da1eb

Do faster checks first

a7bd847

Fix docs parse error

bbf32f0

AlexWaygood reviewed Aug 16, 2024

View reviewed changes

Comment thread crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs Outdated

kbaskett248 and others added 3 commits August 16, 2024 14:17

Extract logic to identify float literal

aadb865

Do faster checks first (again)

d2ca9ea

Improve rule docs

5ddc822

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood reviewed Aug 16, 2024

View reviewed changes

Comment thread crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs Outdated

Simplify

d12efda

Merge remote-tracking branch 'origin/main' into rule/float-literal-de…

812b661

…cimal

AlexWaygood reviewed Aug 18, 2024

View reviewed changes

Use quotes from sylist

401ba8e

MichaReiser enabled auto-merge (squash) August 19, 2024 09:12

style nitpicks

9a8fd54

AlexWaygood force-pushed the rule/float-literal-decimal branch from a4964d9 to 9a8fd54 Compare August 19, 2024 09:17

MichaReiser merged commit f4c8c7e into astral-sh:main Aug 19, 2024

BrewTestBot mentioned this pull request Aug 22, 2024

ruff 0.6.2 Homebrew/homebrew-core#182042

Merged

AlexWaygood mentioned this pull request Nov 15, 2024

New rule: Decimal with float argument #11840

Closed

Conversation

kbaskett248 commented Aug 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

github-actions bot commented Aug 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaReiser Aug 16, 2024

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Aug 16, 2024

Choose a reason for hiding this comment

Uh oh!

kbaskett248 Aug 16, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codspeed-hq bot commented Aug 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #12909 will not alter performance

Summary

Uh oh!

AlexWaygood commented Aug 16, 2024

Uh oh!

kbaskett248 commented Aug 16, 2024

Uh oh!

AlexWaygood Aug 18, 2024

Choose a reason for hiding this comment

Uh oh!

kbaskett248 commented Aug 19, 2024

Uh oh!

AlexWaygood commented Aug 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kbaskett248 commented Aug 15, 2024 •

edited

Loading

github-actions bot commented Aug 15, 2024 •

edited

Loading

`ruff-ecosystem` results

codspeed-hq bot commented Aug 16, 2024 •

edited

Loading