search

dagster-io / dagster

An orchestration platform for the development, production, and observation of data assets.
https://dagster.io
Apache License 2.0
11.91k stars 1.49k forks source link

Update the target Python version for Ruff commands #26094

Closed deepyaman closed 13 hours ago

deepyaman commented 4 days ago

Summary & Motivation

Fix a minor inconsistency. ~There's no effect, because most pyupgrade rules aren't included.~

~(I'd be happy to introduce the pyupgrade rules, if people are open to it.)~

danielgafni commented 3 days ago

@deepyaman hey, looks like ruff actually wants to reformat some files.

❯ make ruff
ruff check --fix .
All checks passed!
ruff format .
27 files reformatted, 4359 files left unchanged

It wasn't kicked off in CI, probably because of the lack of changes in this PR.

As for the UP rule, I am already working on this here: https://github.com/dagster-io/dagster/pull/25703, so I'd suggest to keep the current scope of this PR.

deepyaman commented 3 days ago

@deepyaman hey, looks like ruff actually wants to reformat some files.

❯ make ruff
ruff check --fix .
All checks passed!
ruff format .
27 files reformatted, 4359 files left unchanged

It wasn't kicked off in CI, probably because of the lack of changes in this PR.

Got it. Pre-commit was actually making these changes for me, but I didn't commit them, since I thought something was wrong with my Ruff setup. 🙈 I didn't understand why upgrading the target version would make these changes; I thought maybe Ruff wasn't picking up line-length configuration or something on my end, but seems not.

As for the UP rule, I am already working on this here: #25703, so I'd suggest to keep the current scope of this PR.

Ah, I didn't see this! I've gone ahead and made the requested change, but if you want to just close my PR in favor of yours, that's also understandable!

danielgafni commented 3 days ago

It's fine, there shouldn't be conflicts with my PR! Or they are going to be easy to resolve.