[ruff] New rule float-comparison (RUF067)

[chirizxc]

Summary

Part of #14220

Test Plan

cargo nextest run ruf067

[chirizxc]

We need more tests and cases where the rule can give FP and so on

[chirizxc]

Also, I randomly picked CODE for this rule

[ntBre]

I haven't reviewed, but if there's not a corresponding E723 rule in pycodestyle, I would default to making this a ruff rule. We have a PR in progress for RUF066, so RUF067 would be the next free code.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+1085 -0 violations, +0 -0 fixes in 15 projects; 40 projects unchanged)

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

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

+ .github/tests/test_mr_publish_results.py:63:12: RUF067 Comparison `87.0 == transform_to_seconds("1m27s")` should be replaced by `math.isclose()` or `numpy.isclose()`
+ .github/tests/test_mr_publish_results.py:64:12: RUF067 Comparison `87.3 == transform_to_seconds("1m27.3s")` should be replaced by `math.isclose()` or `numpy.isclose()`
+ .github/tests/test_mr_publish_results.py:65:12: RUF067 Comparison `27.0 == transform_to_seconds("27s")` should be replaced by `math.isclose()` or `numpy.isclose()`
+ .github/tests/test_mr_publish_results.py:66:12: RUF067 Comparison `3627.0 == transform_to_seconds("1h27s")` should be replaced by `math.isclose()` or `numpy.isclose()`
+ .github/tests/test_mr_publish_results.py:67:12: RUF067 Comparison `3687.0 == transform_to_seconds("1h1m27s")` should be replaced by `math.isclose()` or `numpy.isclose()`
+ rasa/core/policies/ensemble.py:57:32: RUF067 Comparison `max_confidence == 0.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ rasa/engine/caching.py:301:16: RUF067 Comparison `self._max_cache_size == 0.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ rasa/shared/core/slots.py:257:16: RUF067 Comparison `x == 1.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ rasa/shared/core/slots.py:260:20: RUF067 Comparison `float(x) == 1.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ rasa/utils/tensorflow/layers.py:291:12: RUF067 Comparison `self.density == 1.0` should be replaced by `math.isclose()` or `numpy.isclose()`
... 25 additional changes omitted for project

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

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

+ airflow-core/tests/unit/api_fastapi/execution_api/versions/head/test_task_instances.py:1225:16: RUF067 Comparison `ti.duration == 3600.00` should be replaced by `math.isclose()` or `numpy.isclose()`
+ airflow-core/tests/unit/cli/commands/test_variable_command.py:205:16: RUF067 Comparison `Variable.get("float", deserialize_json=True) == 42.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ airflow-core/tests/unit/core/test_configuration.py:251:16: RUF067 Comparison `test_conf.getfloat("another", "key8_float", fallback="1") == 1.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ airflow-core/tests/unit/core/test_configuration.py:473:16: RUF067 Comparison `test_conf.getfloat("valid", "key2") == 1.23` should be replaced by `math.isclose()` or `numpy.isclose()`
+ airflow-core/tests/unit/jobs/test_base_job.py:114:20: RUF067 Comparison `most_recent.heartrate == float(job_heartbeat_sec)` should be replaced by `math.isclose()` or `numpy.isclose()`
+ airflow-core/tests/unit/jobs/test_scheduler_job.py:6478:24: RUF067 Comparison `duration == 0.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ airflow-core/tests/unit/jobs/test_scheduler_job.py:6504:24: RUF067 Comparison `duration == 0.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ airflow-core/tests/unit/models/test_pool.py:204:16: RUF067 Comparison `float("inf") == pool.open_slots()` should be replaced by `math.isclose()` or `numpy.isclose()`
+ airflow-core/tests/unit/serialization/serializers/test_serializers.py:129:16: RUF067 Comparison `deserialized_dt.timestamp() == 1657505443.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ airflow-core/tests/unit/serialization/test_dag_serialization.py:4095:16: RUF067 Comparison `result["retry_delay"] == 300.0` should be replaced by `math.isclose()` or `numpy.isclose()`
... 15 additional changes omitted for project

apache/superset (+20 -0 violations, +0 -0 fixes)

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

+ tests/integration_tests/utils_tests.py:394:16: RUF067 Comparison `cast_to_num("5.2") == 5.2` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/integration_tests/utils_tests.py:396:16: RUF067 Comparison `cast_to_num(10.1) == 10.1` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/unit_tests/commands/databases/csv_reader_test.py:853:12: RUF067 Comparison `df.iloc[0]["Score"] == 95.5` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/unit_tests/commands/databases/csv_reader_test.py:950:12: RUF067 Comparison `result_df.iloc[0]["score"] == 95.5` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/unit_tests/extensions/ssh_test.py:35:12: RUF067 Comparison `sshtunnel.TUNNEL_TIMEOUT == 123.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/unit_tests/extensions/ssh_test.py:36:12: RUF067 Comparison `sshtunnel.SSH_TIMEOUT == 321.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/unit_tests/utils/json_tests.py:227:12: RUF067 Comparison `json.base_json_conv(Decimal("1.0")) == 1.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/unit_tests/utils/json_tests.py:42:12: RUF067 Comparison `data["float"] == 0.12345` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/unit_tests/utils/json_tests.py:86:12: RUF067 Comparison `reloaded_data["float"] == 0.12345` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/unit_tests/utils/webdriver_test.py:209:16: RUF067 Comparison `call_kwargs["timeout"] == 30.0` should be replaced by `math.isclose()` or `numpy.isclose()`
... 10 additional changes omitted for project

bokeh/bokeh (+199 -0 violations, +0 -0 fixes)

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

+ examples/topics/categorical/slope_graph.py:29:10: RUF067 Comparison `df["year"] == 2000.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ examples/topics/categorical/slope_graph.py:29:35: RUF067 Comparison `df["year"] == 2010.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ src/bokeh/colors/color.py:321:12: RUF067 Comparison `self.a == 1.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ src/bokeh/colors/color.py:460:12: RUF067 Comparison `self.a == 1.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/integration/tools/test_box_zoom_tool.py:106:16: RUF067 Comparison `(results['xrstart'] + results['xrend'])/2.0 == pytest.approx(0.25)` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/integration/tools/test_box_zoom_tool.py:107:16: RUF067 Comparison `(results['yrstart'] + results['yrend'])/2.0 == pytest.approx(0.75)` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/integration/tools/test_range_tool.py:103:16: RUF067 Comparison `results['start'] == 0.4` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/integration/tools/test_range_tool.py:104:16: RUF067 Comparison `results['end'] == 0.6` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/integration/tools/test_range_tool.py:118:16: RUF067 Comparison `results['start'] == 0.5` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/integration/tools/test_range_tool.py:119:16: RUF067 Comparison `results['end'] == 0.7` should be replaced by `math.isclose()` or `numpy.isclose()`
... 189 additional changes omitted for project

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

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

+ ibis/backends/bigquery/tests/system/test_client.py:434:48: RUF067 Comparison `t.max != 9999.9` should be replaced by `math.isclose()` or `numpy.isclose()`
+ ibis/backends/bigquery/tests/system/udf/test_udf_execute.py:102:12: RUF067 Comparison `con.execute(expr) == 8.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ ibis/backends/clickhouse/tests/test_operators.py:175:12: RUF067 Comparison `round(con.execute(expr), 3) == -5.245` should be replaced by `math.isclose()` or `numpy.isclose()`
+ ibis/backends/druid/tests/conftest.py:58:31: RUF067 Comparison `js[datasource] == 100.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ ibis/backends/duckdb/tests/test_udf.py:180:12: RUF067 Comparison `result.iat[0] == 1.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ ibis/backends/postgres/tests/test_client.py:515:12: RUF067 Comparison `value[0].as_py() == 1.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ ibis/backends/sqlite/tests/test_client.py:55:12: RUF067 Comparison `result == 0.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ ibis/backends/tests/test_map.py:504:12: RUF067 Comparison `con.execute(expr) == 3.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ ibis/backends/tests/test_numeric.py:1126:12: RUF067 Comparison `result == 0.5` should be replaced by `math.isclose()` or `numpy.isclose()`
+ ibis/common/tests/test_annotations.py:371:12: RUF067 Comparison `test(2, 3.0) == 6.0` should be replaced by `math.isclose()` or `numpy.isclose()`
... 20 additional changes omitted for project

langchain-ai/langchain (+31 -0 violations, +0 -0 fixes)

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

+ libs/core/tests/unit_tests/language_models/chat_models/test_base.py:1213:12: RUF067 Comparison `ls_params["ls_temperature"] == 0.2` should be replaced by `math.isclose()` or `numpy.isclose()`
+ libs/core/tests/unit_tests/language_models/llms/test_base.py:273:12: RUF067 Comparison `ls_params["ls_temperature"] == 0.2` should be replaced by `math.isclose()` or `numpy.isclose()`
+ libs/core/tests/unit_tests/rate_limiters/test_in_memory_rate_limiter.py:104:16: RUF067 Comparison `rate_limiter.available_tokens == 199.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ libs/core/tests/unit_tests/rate_limiters/test_in_memory_rate_limiter.py:107:16: RUF067 Comparison `rate_limiter.available_tokens == 499.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ libs/core/tests/unit_tests/rate_limiters/test_in_memory_rate_limiter.py:21:12: RUF067 Comparison `rate_limiter.available_tokens == 0.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ libs/core/tests/unit_tests/rate_limiters/test_in_memory_rate_limiter.py:36:16: RUF067 Comparison `rate_limiter.available_tokens == 1.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ libs/core/tests/unit_tests/rate_limiters/test_in_memory_rate_limiter.py:64:16: RUF067 Comparison `rate_limiter.available_tokens == 1.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ libs/core/tests/unit_tests/rate_limiters/test_in_memory_rate_limiter.py:85:16: RUF067 Comparison `rate_limiter.available_tokens == 199.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ libs/core/tests/unit_tests/rate_limiters/test_in_memory_rate_limiter.py:88:16: RUF067 Comparison `rate_limiter.available_tokens == 499.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ libs/langchain/tests/unit_tests/agents/test_agent_iterator.py:294:12: RUF067 Comparison `agent_iter.time_elapsed == 0.0` should be replaced by `math.isclose()` or `numpy.isclose()`
... 21 additional changes omitted for project

lnbits/lnbits (+1 -0 violations, +0 -0 fixes)

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

+ tests/unit/test_services_fees.py:61:12: RUF067 Comparison `fee / 1000 == 199` should be replaced by `math.isclose()` or `numpy.isclose()`

milvus-io/pymilvus (+3 -0 violations, +0 -0 fixes)

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

+ examples/orm_deprecated/bulk_import/example_bulkwriter_with_nullable.py:300:16: RUF067 Comparison `len(b[0]) == DIM/8` should be replaced by `math.isclose()` or `numpy.isclose()`
+ pymilvus/orm/iterator.py:556:12: RUF067 Comparison `self._width == 0.0` should be replaced by `math.isclose()` or `numpy.isclose()`
+ tests/test_search_result.py:105:16: RUF067 Comparison `first_hit["distance"] == 0.` should be replaced by `math.isclose()` or `numpy.isclose()`

... Truncated remaining completed project reports due to GitHub comment length restrictions

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
RUF067	1085	1085	0	0	0

[chirizxc]

• examples/topics/categorical/slope_graph.py:29:10: RUF067 Comparison df["year"] == 2000.0 should be replaced by math.isclose(df["year"], 2000.0, rel_tol=1e-09, abs_tol=1e-09)
• examples/topics/categorical/slope_graph.py:29:35: RUF067 Comparison df["year"] == 2010.0 should be replaced by math.isclose(df["year"], 2010.0, rel_tol=1e-09, abs_tol=1e-09)

This breaks the behavior of code and results in:

TypeError: cannot convert the series to <class 'float'>

[chirizxc]

What should we do in such a case? Should we somehow mark this fact in the documentation or what should we do, because we will not be able to check in general that such calls will not lead to some code breakages

[chirizxc]

The rule detects potentially problematic float comparisons correctly, but we should not suggest replacing only math.isclose() but also numpy.isclose()

[ntBre]

Would it help with the false positives, and maybe with the numpy cases, to apply the rule only in boolean contexts? I think we have other rules that apply only to expressions used as if conditions or in negated not expressions, for example. We even have a semantic model helper for it:

ruff/crates/ruff_python_semantic/src/model.rs

Line 1974 in 5f136c2

pub const fn in_boolean_test(&self) -> bool {

This rule is going to require type inference to get exactly right anyway, so this might be a good way to be more conservative in the meantime.

I also only gave the implementation a quick skim and could have overlooked this, but does this rule apply to comparisons like <= and > too? I would probably expect those to be allowed since something like delta < 1e-8 should work as expected, I'm assuming that's what math.isclose does under the hood. Along those lines, we may want to rename the rule to float-equality-comparison, if it only applies to equality.

[chirizxc]

Would it help with the false positives, and maybe with the numpy cases, to apply the rule only in boolean contexts? I think we have other rules that apply only to expressions used as if conditions or in negated not expressions, for example. We even have a semantic model helper for it:

ruff/crates/ruff_python_semantic/src/model.rs

Line 1974 in 5f136c2

pub const fn in_boolean_test(&self) -> bool {

This rule is going to require type inference to get exactly right anyway, so this might be a good way to be more conservative in the meantime.

I also only gave the implementation a quick skim and could have overlooked this, but does this rule apply to comparisons like <= and > too? I would probably expect those to be allowed since something like delta < 1e-8 should work as expected, I'm assuming that's what math.isclose does under the hood. Along those lines, we may want to rename the rule to float-equality-comparison, if it only applies to equality.

Yes, this only applies to == and !=, just as it works in SonarQube.