Renovate quality tools #1421
Renovate quality tools #1421
Conversation
There was a problem hiding this comment.
Thanks a lot for this update, which looks indeed very useful.
Doing a full review is a bit difficult because the diff is so large. Could we maybe add some exceptions to the ruff checks for the time being? Specifically, I'm thinking of:
- https://docs.astral.sh/ruff/rules/redundant-open-modes/
- https://docs.astral.sh/ruff/rules/utf8-encoding-declaration/
- https://docs.astral.sh/ruff/rules/f-string/
This should take care of the majority of changes and thus lead to a much smaller diff and make the review easier. Then I can discuss with my co-maintainers if we want to have these exceptions or not, and if we don't, remove them in a later PR.
|
@BenjaminBossan Sure! I rebased this and split the single |
src/peft/auto.py
Outdated
| @@ -50,7 +49,7 @@ class _BaseAutoPeftModel: | |||
|
|
|||
| def __init__(self, *args, **kwargs): | |||
| # For consistency with transformers: https://github.com/huggingface/transformers/blob/91d7df58b6537d385e90578dac40204cb550f706/src/transformers/models/auto/auto_factory.py#L400 | |||
| raise EnvironmentError( | |||
| raise OSError( | |||
There was a problem hiding this comment.
As the comment above suggests, let's stick with EnvironmentError, even if it's not the correct error type here.
There was a problem hiding this comment.
I believe the "consistency" there refers to raising an error in __init__, not the particular error type, but sure, added a commit to noqa that instead.
There was a problem hiding this comment.
No, this refers to the type of error. I know because I flagged the error type initially :)
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
src/peft/auto.py
Outdated
| @@ -50,7 +49,7 @@ class _BaseAutoPeftModel: | |||
|
|
|||
| def __init__(self, *args, **kwargs): | |||
| # For consistency with transformers: https://github.com/huggingface/transformers/blob/91d7df58b6537d385e90578dac40204cb550f706/src/transformers/models/auto/auto_factory.py#L400 | |||
| raise EnvironmentError( | |||
| raise OSError( | |||
There was a problem hiding this comment.
No, this refers to the type of error. I know because I flagged the error type initially :)
|
@BenjaminBossan Done, rebased, removed a couple of ignores by either autofixing or noqa'ing (there are 29 functions that are too complex according to default C901, so not touching those). |
In PR huggingface#1421, ruff was extended to check all directories. This is fine for those directories that come with PEFT. However, developers may have other local directories that they do not want to be checked. Therefore, it is better to list the directories to be checked rather than checking all.
In PR #1421, ruff was extended to check all directories. This is fine for those directories that come with PEFT. However, developers may have other local directories that they do not want to be checked. Therefore, it is better to list the directories to be checked rather than checking all.
In PR huggingface#1421, ruff was extended to check all directories. This is fine for those directories that come with PEFT. However, developers may have other local directories that they do not want to be checked. Therefore, it is better to list the directories to be checked rather than checking all.
This PR:
isortconfiguration that hasn't been in use since 9990189blackformatting withruff format(thoughblackis still a hidden dependency viahf-doc-builderUP(upgrade) rules and auto-fixes what can be auto-fixed.This is similar to e.g. huggingface/transformers#27144 and huggingface/diffusers#5841.