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

Import initial CI configuration #7

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

russellb
Copy link
Member

@russellb russellb commented May 1, 2024

  • Import initial CI configuration
  • Apply tox -e ruff formatting changes
  • Remove unused Python imports
  • Resolve lint warnings about redefining variables
  • Resolve lint warnings about unused arguments
  • Make pylint complete successfully
  • README.md: Drop "BACKEND" from the title

commit e489e17
Author: Russell Bryant rbryant@redhat.com
Date: Wed May 1 07:48:01 2024 -0400

Import initial CI configuration

Import jobs to run

- linting of github workflows (actionlint)
- python formatting and linting
- spell checking
- markdown linting

This also includes requirements*.txt files.

Signed-off-by: Russell Bryant <rbryant@redhat.com>

commit 76d83f0
Author: Russell Bryant rbryant@redhat.com
Date: Wed May 1 07:48:22 2024 -0400

Apply `tox -e ruff` formatting changes

Signed-off-by: Russell Bryant <rbryant@redhat.com>

commit 29fe072
Author: Russell Bryant rbryant@redhat.com
Date: Wed May 1 07:55:07 2024 -0400

Remove unused Python imports

Signed-off-by: Russell Bryant <rbryant@redhat.com>

commit e88162f
Author: Russell Bryant rbryant@redhat.com
Date: Wed May 1 08:02:24 2024 -0400

Resolve lint warnings about redefining variables

For example:

```
main_ds.py:50:16: W0621: Redefining name 'args' from outer scope (line 280) (redefined-outer-name)
main_ds.py:100:10: W0621: Redefining name 'args' from outer scope (line 280) (redefined-outer-name)
main_ds.py:188:9: W0621: Redefining name 'args' from outer scope (line 280) (redefined-outer-name)
tokenizer_utils.py:38:24: W0621: Redefining name 'SPECIAL_TOKENS' from outer scope (line 22) (redefined-outer-name)
tokenizer_utils.py:38:55: W0621: Redefining name 'CHAT_TEMPLATE' from outer scope (line 24) (redefined-outer-name)
data_process.py:173:9: W0621: Redefining name 'args' from outer scope (line 271) (redefined-outer-name)
```

Signed-off-by: Russell Bryant <rbryant@redhat.com>

commit b6adf5d
Author: Russell Bryant rbryant@redhat.com
Date: Wed May 1 08:05:24 2024 -0400

Resolve lint warnings about unused arguments

By prefixing the unused arguments with an underscore, pylint
understands this as a hint that we intentionally ignore the argument.

Signed-off-by: Russell Bryant <rbryant@redhat.com>

commit 45003e1
Author: Russell Bryant rbryant@redhat.com
Date: Wed May 1 08:12:21 2024 -0400

Make pylint complete successfully

Drop a couple of f-strings without any interpolation, set an encoding
type on `open()`, and update the pylint configuration to ignore all
remaining warnings for now.

Signed-off-by: Russell Bryant <rbryant@redhat.com>

commit 851d73a
Author: Russell Bryant rbryant@redhat.com
Date: Mon Apr 29 10:08:19 2024 -0400

README.md: Drop "BACKEND" from the title

This code is not backend-specific, so drop it from the readme title.
While we're at it, capitalize DeepSpeed as the upstream project does
it.

While we're at it, add badges to README.

Signed-off-by: Russell Bryant <rbryant@redhat.com>

@russellb
Copy link
Member Author

russellb commented May 1, 2024

The jobs are running on my fork, so you can see them pass here: https://github.com/russellb/instructlab-training/actions

This is split up into separate commits to make it easier to review. The bulk import of config that doesn't touch code is the first commit. Code changes are split apart so you can more easily see changes from the formatting tool vs "real" code changes, along with their explanations.

Copy link
Member

@nathan-weinberg nathan-weinberg left a comment

Choose a reason for hiding this comment

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

LGTM

Import jobs to run

- linting of github workflows (actionlint)
- python formatting and linting
- spell checking
- markdown linting

This also includes requirements*.txt files.

Signed-off-by: Russell Bryant <rbryant@redhat.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
For example:

```
main_ds.py:50:16: W0621: Redefining name 'args' from outer scope (line 280) (redefined-outer-name)
main_ds.py:100:10: W0621: Redefining name 'args' from outer scope (line 280) (redefined-outer-name)
main_ds.py:188:9: W0621: Redefining name 'args' from outer scope (line 280) (redefined-outer-name)
tokenizer_utils.py:38:24: W0621: Redefining name 'SPECIAL_TOKENS' from outer scope (line 22) (redefined-outer-name)
tokenizer_utils.py:38:55: W0621: Redefining name 'CHAT_TEMPLATE' from outer scope (line 24) (redefined-outer-name)
data_process.py:173:9: W0621: Redefining name 'args' from outer scope (line 271) (redefined-outer-name)
```

Signed-off-by: Russell Bryant <rbryant@redhat.com>
By prefixing the unused arguments with an underscore, pylint
understands this as a hint that we intentionally ignore the argument.

Signed-off-by: Russell Bryant <rbryant@redhat.com>
Drop a couple of f-strings without any interpolation, set an encoding
type on `open()`, and update the pylint configuration to ignore all
remaining warnings for now.

Signed-off-by: Russell Bryant <russell.bryant@gmail.com>
This code is not backend-specific, so drop it from the readme title.
While we're at it, capitalize DeepSpeed as the upstream project does
it.

While we're at it, add badges to README.

Signed-off-by: Russell Bryant <rbryant@redhat.com>
@russellb
Copy link
Member Author

russellb commented May 2, 2024

closed by accident

@russellb russellb reopened this May 2, 2024
@nathan-weinberg
Copy link
Member

@russellb think we should revisit this for the librarification?

@russellb
Copy link
Member Author

@russellb think we should revisit this for the librarification?

imo yes

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.

None yet

2 participants