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

Automated formatting #1180

Merged
merged 3 commits into from
Mar 24, 2024
Merged

Conversation

Andrei-Aksionov
Copy link
Contributor

@Andrei-Aksionov Andrei-Aksionov commented Mar 22, 2024

Hi there 👋

Another round of code-style fixes:

ruff check * \
    --select E,W,F,S \
    --extend-select ANN001,ANN201,ANN205,ANN206,ARG001,C4,I001,PT,RET,SIM \
    --ignore C408,E402,E501,E731,PT004,PT007,S101,S108,S113,S310,S603 \
    --per-file-ignores "*/**/__init__.py":I001,"*/**/__init__.py":F401,"tests/*":ANN,"*/**/unsloth/kernels/*":ALL \
    --line-length 120 \
    --target-version py38 \
    --fix-only; \
black \
    -l120 \
    -C \
    --preview \
    --target-version py38 \
    config_hub eval extensions litgpt tests \
    --exclude unsloth/kernels

Notable changes:

  • disabled F401 (imported but unused) for __init__.py files.
  • excluded unsloth/kernels from linting.
  • added extensions folder to the list.

For those preferring pre-commit I updated a corresponding config.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kernels directory shouldnt be formatted to keep the original style of the code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops.
Fixed.
+ updated CLI command in the PR description.

@Borda
Copy link
Member

Borda commented Mar 24, 2024

Why not add the pre-commit or at least write this configuration to the already existing pyproject.toml?

@carmocca carmocca merged commit 9023c45 into Lightning-AI:main Mar 24, 2024
7 of 8 checks passed
@Andrei-Aksionov Andrei-Aksionov deleted the automated_formatting branch March 25, 2024 10:44
@Andrei-Aksionov
Copy link
Contributor Author

@Borda when Lit-GPT was a set of scripts it was not the option, since the decision was made to not have such config files in the repo (I believe for the sake of simplicity).
But now we have LitGPT, so I think we can revisit this question. @lantiga?

@Borda
Copy link
Member

Borda commented Mar 25, 2024

as the pyproject already exists, I would rather have the configuration there than in some PR description which may differ over time...

rasbt pushed a commit that referenced this pull request Apr 3, 2024
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 this pull request may close these issues.

3 participants