Skip to content

[flake8-datetimez] Stabilize datetime-min-max (DTZ901)#16635

Merged
ntBre merged 1 commit intomicha/ruff-0.10from
brent/dtz901-0.10
Mar 12, 2025
Merged

[flake8-datetimez] Stabilize datetime-min-max (DTZ901)#16635
ntBre merged 1 commit intomicha/ruff-0.10from
brent/dtz901-0.10

Conversation

@ntBre
Copy link
Copy Markdown
Contributor

@ntBre ntBre commented Mar 11, 2025

Summary

Stabilizes DTZ901, renames the rule function to match the rule name, removes the preview_rules test, and handles some nits in the docs (mention min first to match the rule name too).

Test Plan

1 closed issue on 2024-11-12, 4 days after the rule was added. No issues since

Summary
--

Stabilizes DTZ901, renames the rule function to match the rule name, removes the
`preview_rules` test, and handles some nits in the docs (mention `min` first to
match the rule name too).

Test Plan
--

1 closed issue on 2024-11-12, 4 days after the rule was added. No issues since
@ntBre ntBre added the rule Implementing or modifying a lint rule label Mar 11, 2025
@ntBre ntBre added this to the v0.10 milestone Mar 11, 2025
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Mar 11, 2025

CodSpeed Performance Report

Merging #16635 will degrade performances by 12.88%

Comparing brent/dtz901-0.10 (5cae870) with micha/ruff-0.10 (5ec7c13)

Summary

❌ 1 regressions
✅ 31 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
red_knot_check_file[incremental] 4.8 ms 5.5 ms -12.88%

@github-actions
Copy link
Copy Markdown
Contributor

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+11 -0 violations, +0 -0 fixes in 1 projects; 54 projects unchanged)

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

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

+ airflow/example_dags/example_inlet_event_extra.py:38:16: DTZ901 Use of `datetime.datetime.min` without timezone information
+ airflow/example_dags/example_inlet_event_extra.py:53:16: DTZ901 Use of `datetime.datetime.min` without timezone information
+ airflow/example_dags/example_outlet_event_extra.py:39:16: DTZ901 Use of `datetime.datetime.min` without timezone information
+ airflow/example_dags/example_outlet_event_extra.py:53:16: DTZ901 Use of `datetime.datetime.min` without timezone information
+ airflow/example_dags/example_outlet_event_extra.py:67:16: DTZ901 Use of `datetime.datetime.min` without timezone information
+ providers/google/src/airflow/providers/google/cloud/operators/gcs.py:805:46: DTZ901 Use of `datetime.datetime.max` without timezone information
+ providers/google/src/airflow/providers/google/cloud/transfers/mysql_to_gcs.py:137:26: DTZ901 Use of `datetime.datetime.min` without timezone information
+ tests/cli/commands/remote_commands/test_dag_command.py:351:37: DTZ901 Use of `datetime.datetime.max` without timezone information
+ tests/dag_processing/test_manager.py:495:75: DTZ901 Use of `datetime.datetime.min` without timezone information
+ tests/dag_processing/test_manager.py:513:75: DTZ901 Use of `datetime.datetime.max` without timezone information
+ tests/models/test_dag.py:2442:24: DTZ901 Use of `datetime.datetime.min` without timezone information

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
DTZ901 11 11 0 0 0

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre
Copy link
Copy Markdown
Contributor Author

ntBre commented Mar 11, 2025

I think the ecosystem check revealed three false positives:

https://github.com/apache/airflow/blob/eebdf4e9fbbe9badd1f93dbaf1c9a0d1d2aa357c/tests/cli/commands/remote_commands/test_dag_command.py#L350-L351

https://github.com/apache/airflow/blob/eebdf4e9fbbe9badd1f93dbaf1c9a0d1d2aa357c/tests/dag_processing/test_manager.py#L494-L496

https://github.com/apache/airflow/blob/eebdf4e9fbbe9badd1f93dbaf1c9a0d1d2aa357c/tests/dag_processing/test_manager.py#L512-L514

I don't think there's much we can do here because timezone.make_aware is defined in the airflow.utils module, but I think it's a false positive because it clearly sounds like make_aware is addressing the fact that "datetime.max and datetime.min are non-timezone-aware datetime objects" from our docs. We'd probably need some kind of configuration allowlist to handle that.

@ntBre ntBre mentioned this pull request Mar 12, 2025
2 tasks
@UnknownPlatypus
Copy link
Copy Markdown
Contributor

Django also have a timezone.make_aware util so you might encounter false positive in django code bases too.

You could maybe let the rule pass if it's wrapped in any timezone.make_aware call ?

Copy link
Copy Markdown
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Airflow would have to use a noqa comment in this case which is an explicit acknowledgement that they handled the no-timezone case.

This is also not ap roblem specific to DTZ901 (as far as I understand). E.g. https://docs.astral.sh/ruff/rules/call-date-today/ has the same problem that we flag

import datetime

timezone.make_aware(datetime.datetime.today())

@ntBre
Copy link
Copy Markdown
Contributor Author

ntBre commented Mar 12, 2025

Sounds good, moving forward with stabilization.

@ntBre ntBre merged commit 2cada23 into micha/ruff-0.10 Mar 12, 2025
20 of 21 checks passed
@ntBre ntBre deleted the brent/dtz901-0.10 branch March 12, 2025 18:43
MichaReiser pushed a commit that referenced this pull request Mar 13, 2025
Summary
--

Stabilizes DTZ901, renames the rule function to match the rule name,
removes the `preview_rules` test, and handles some nits in the docs
(mention `min` first to match the rule name too).

Test Plan
--

1 closed issue on 2024-11-12, 4 days after the rule was added. No issues
since
MichaReiser pushed a commit that referenced this pull request Mar 13, 2025
Summary
--

Stabilizes DTZ901, renames the rule function to match the rule name,
removes the `preview_rules` test, and handles some nits in the docs
(mention `min` first to match the rule name too).

Test Plan
--

1 closed issue on 2024-11-12, 4 days after the rule was added. No issues
since
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

Successfully merging this pull request may close these issues.

3 participants