-
Notifications
You must be signed in to change notification settings - Fork 154
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
[Bot] pre-commit autoupdate #674
Conversation
updates: - [github.com/astral-sh/ruff-pre-commit: v0.5.6 → v0.6.3](astral-sh/ruff-pre-commit@v0.5.6...v0.6.3) - [github.com/pre-commit/mirrors-mypy: v1.10.1 → v1.11.2](pre-commit/mirrors-mypy@v1.10.1...v1.11.2) - [github.com/errata-ai/vale: v3.7.0 → v3.7.1](errata-ai/vale@v3.7.0...v3.7.1)
for more information, see https://pre-commit.ci
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.
@maxschulz-COL @lingyielia - could you finish this PR off?
I've reverted the mypy version, because we cannot update as long as we don't use Pydantic V2. With the latest version of ruff, lots of linting errors are raised from the notebooks inside vizro-ai.
I think some of them you can probably safely ignore (prints), others might be worth implementing (docstrings / line length). So either push a change to ignore certain rules or fix the linting errors :)
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.
I assume the dashboard changes are purely stylistic? In which case happy to merge
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.
@lingyielia - I am sure it's possible, but could you check if one can ignore certain rules in ruff per files?
Because, essentially you don't want to add an ignore in each line, but rather ignore that rule for all notebooks inside vizro-ai. For example the print rule. If not easy to implement, then happy to merge as is 👍
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.
Looks a lot cleaner now! 🚀 Thanks!!
updates: