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

Switch to ruff #142

Closed
adamgayoso opened this issue Jan 13, 2023 · 14 comments · Fixed by #147
Closed

Switch to ruff #142

adamgayoso opened this issue Jan 13, 2023 · 14 comments · Fixed by #147

Comments

@adamgayoso
Copy link
Member

No description provided.

@ivirshup
Copy link
Member

For context: https://github.com/charliermarsh/ruff

@grst grst mentioned this issue Jan 25, 2023
@ivirshup
Copy link
Member

ivirshup commented Jan 30, 2023

This has been discussed more extensively on the zulip

@flying-sheep
Copy link
Member

flying-sheep commented Jan 31, 2023

One thing to note is that Ruff is in a (temporary) pickle regarding the new Python parser: astral-sh/ruff#282. In short, no fast libraries seem to exist that can parse the new match statement into a Concrete Syntax Tree (CST). I’m sure that’ll be fixed in due time, but if you want to use Ruff on a 3.10+ codebase today, you can’t use match yet.

@flying-sheep
Copy link
Member

flying-sheep commented Feb 21, 2023

That issue is well on its way, RustPython will merge match support today (RustPython/RustPython#4519) and there’s already a PR depending on that support to add match support in ruff (astral-sh/ruff#3047)

So there’s nothing keeping us from using ruff! By the time someone will create a new project using this template and add a match statement, support should be complete.

@flying-sheep flying-sheep mentioned this issue Feb 21, 2023
7 tasks
@adamgayoso
Copy link
Member Author

It would be great to add this one after

#141

@flying-sheep
Copy link
Member

flying-sheep commented Feb 22, 2023

Since @grst suggested using pyupgrade directly instead of the Ruff version, I felt like recapping why using as many checks from Ruff as possible is a good idea:

  • It’s very fast as it only parses once and then runs all checks on the parse tree (so we can run all checks we feel like running without adding noticable runtime, and should use the ruff version of everything we want to use)
  • It has support for many flake8 plugins and other linters we use, including pydocstyle and pyupgrade
  • It has autofixes for all check where that’s possible, we don’t need to rely on Fixit or autoflake to implement rules (I guess autoflake misses fixes especially for plugin rules)
  • There’s a unified rule namespace, and all rules and config options are documented in one place(!)

@grst
Copy link
Collaborator

grst commented Feb 22, 2023

There's either a typo or a misunderstanding. I'm all for using the ruff version of pyupgrade!

@ivirshup
Copy link
Member

Just to clarify, is there a suggestion here that we'd bundle enabling more checks with switching to ruff?

I'd want to stick with just checks and fixes we currently use when using ruff.

@Zethson
Copy link
Member

Zethson commented Feb 22, 2023

I don't see why we shouldn't add more stuff when we're switching anyways. There's no need to wait for anything here or is there?

@grst
Copy link
Collaborator

grst commented Feb 22, 2023

I think we can all agree that some checks are more important than others.

Back then in Munich, the argument was to not include less important checks to keep the template less intimidating for new developers.

Or do all of the newly suggested checks have autofixes? I'm not sure how that would work for naming conventions for instance.

@ivirshup
Copy link
Member

Back then in Munich, the argument was to not include less important checks to keep the template less intimidating for new developers.

+1, definitely want to keep this simple.

I also don't think adding ruff should have to bring up new discussions about which checks are good or not. We already spent a lot of time on this, and came up with a big set. Any changes to this set can easily be done separately.

Or do all of the newly suggested checks have autofixes?

I think ruff has focussed on things with auto fixes, though not all things have auto fixes. In practice, I think this meant prioritizing implementing features from existing tools like isort and autoflake, and de-prioritizing things that just error. I don't think it has many features that haven't been implemented elsewhere (here's what's documented as ruff specific).

@grst
Copy link
Collaborator

grst commented Feb 22, 2023

To add to this, I believe the template should be default include the "minimal set of checks that everyone should use" and not "what some some of use want to use".

After all, it's very easy to customize the checks after an instance of the repo was created.

@Zethson
Copy link
Member

Zethson commented Feb 22, 2023

Fair enough :) Agreed.

@flying-sheep
Copy link
Member

I agree! Let’s continue discussion of checks in #149. I think Ruff has a few more checks that fit our selection criteria mentioned there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants