-
Notifications
You must be signed in to change notification settings - Fork 92
Onboarding #1
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
Onboarding #1
Conversation
* `run` & `fix` commands are added to the Makefile * `pyproject.toml` now includes more specifications for code quality tools
* `create_backend` is no longer fail due to inproper condition * rename: `BackendTypes` -> `BackendType`
Makefile
Outdated
|
|
||
| .PHONY: install install-dev quality style test test-unit test-integration test-e2e test-smoke test-sanity test-regression build clean | ||
|
|
||
| fix: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@parfeniukink this is what style is supposed to be for. Can you migrate these pieces under style so we can unify that?
Also, black and ruff we're disagreeing on how to format some edge cases around indexing lists, so let's remove the call into black completely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markurtz
My TLDR: consider using isort+black instead of ruff. Just some of my thought. But I will remove it as you wish.
Detailed
sure, but are you sure about the ruff? I mean.. I know that it is a great tool in terms of linting / formatting and even suppose to be an LSP ( a bit worst than pyright nowadays, but anyways )
Maybe we should consider using ruff only for linting ( since it is faster than flake8, pylint, etc ) and black for formatting. The black in known as uncompromising formatter and they specialized only on formatting when ruff is actually was designed as a linter. I just mean that the formatting is a secondary concern, which might mean it doesn't enforce formatting as strictly or comprehensively as black.
Another reason behind is that the ruff also sorts imports but isort makes it a bit better.
ruff latest release version is 0.4.10 for today and just wanted to notice that they only copied most of the code from other linters like flake8 so the core functionality is a bit stolen 😄
What about authors of all these tools?
- Python Code Quality Authority for
isort - Python Software Foundation for
black
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@parfeniukink is definitely a good point to bring up. According to their docs, Ruff is supposed to have full feature parity with flake8, black, and isort.
The big push behind ruff from my side is ideally to standardize to a single tool across our full stack and ML applications that is backed by the top libraries we use / will integrate with, which are specifically Hugging Face Transformers and FastAPI.
If we can get black working with Ruff, I'm in favor of keeping it, but I do want to ensure the base authority is Ruff due to its performance, the increasing adoption, and the downstream effects of that choice.
No description provided.