Improve Makefile and code quality #62
Conversation
.github/workflows/tests.yml
Outdated
| pip install .[dev] | ||
| - name: Check quality | ||
| run: | | ||
| black --check --line-length 119 --target-version py36 tests trl |
There was a problem hiding this comment.
If I'm not mistaken, you can just run make quality here. this has the advantage of having a single source of truth for the quality logic
.github/workflows/tests.yml
Outdated
| - name: Set up Python | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.7" |
There was a problem hiding this comment.
see my comment below about considering v3.8 as the minimum version since this one will be eol in 6 months
| transformers_model_from_save.state_dict()[key], transformers_model.state_dict()[key] | ||
| ) | ||
| ) | ||
|
|
||
|
|
||
| class VHeadModelTester(BaseModelTester, unittest.TestCase): |
There was a problem hiding this comment.
not for this pr, but a small nit that being explicit with e.g. ValueHeadModelTester is generally best practice + enables newcomers to easier understand the codebase.
the same nit applies to docstrings using v-head vs value head and filenames like modeling_vhead.py being better named as modeling_value_head.py
| @@ -83,25 +90,25 @@ def test_vhead(self): | |||
| for model_name in self.all_model_names: | |||
| model = AutoModelForCausalLMWithValueHead.from_pretrained(model_name) | |||
| self.assertTrue(hasattr(model, "v_head")) | |||
|
|
|||
| def test_vhead_nb_classes(self): | |||
There was a problem hiding this comment.
would this be better?
| def test_vhead_nb_classes(self): | |
| def test_vhead_shape(self): |
There was a problem hiding this comment.
I'd address this too in a separate PR.
Co-authored-by: lewtun <lewis.c.tunstall@gmail.com>
|
The documentation is not available anymore as the PR was closed or merged. |
|
All comments should be addressed. Also applied the quality to the recent merges. |
This PR improves Makefile commands (borrowing from evaluate/datasets) and adds a code quality CI. The last commit fixes the existing code quality issues.