Fix formatting/typing so pre-commit hooks pass #1451
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ianmckenzie-oai
requested review from
andrew-openai,
jwang47,
logankilpatrick,
etr2460 and
katyhshi
as code owners
ianmckenzie-oai
force-pushed
the
ian/fix-precommit
branch
from
d5035fc
to
8dff1b5
Compare
etr2460
approved these changes
Jan 10, 2024
Linmj-Judy
pushed a commit
to TablewareBox/evals
that referenced
this pull request
Feb 27, 2024
(Not an eval)
**One-line summary**: Pre-commit hooks were failing. I identified the
main cause, and then fixed all secondary pre-commit issues. I only
changed the logic in one place, `oiaevalset.py`.
I was having issues with type-hinting and identified that the old
`typings` directory was causing the `from openai import OpenAI` import
to register as an error. I decided to go through and fix all the issues
that appeared in `pre-commit run --all-files`.
NOTE:
- I changed the logic in `oaievalset.py` by adding a `continue`
statement if an `eval` or `eval.key` was missing.
- As far as I can tell this should basically never happen, but is
correct behavior.
- Another option would be to assert that `eval` and `eval.key` are not
`None` but forcing an error here doesn't match what I interpret as
intended behavior.
The manual work involved was mainly:
1. Deleting the `typings` directory, which was interfering with `openai`
type-hints (such as `from openai import OpenAI`)
2. Fixing type issues in `oaievalset.py`.
3. Moving the `client =
OpenAI(api_key=os.environ.get("OPENAI_API_KEY"))` line below all the
imports.
4. Breaking lines of length >767 into smaller chunks using line
continuation.
Thus this PR is broken into three parts:
1. Deleting `typings` (first commit)
2. Manually cleaning up issues (middle commits)
3. Applying autofixes from the pre-commit hooks (last commit)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(Not an eval)
One-line summary: Pre-commit hooks were failing. I identified the main cause, and then fixed all secondary pre-commit issues. I only changed the logic in one place,
oiaevalset.py.I was having issues with type-hinting and identified that the old
typingsdirectory was causing thefrom openai import OpenAIimport to register as an error. I decided to go through and fix all the issues that appeared inpre-commit run --all-files.NOTE:
oaievalset.pyby adding acontinuestatement if anevaloreval.keywas missing.evalandeval.keyare notNonebut forcing an error here doesn't match what I interpret as intended behavior.The manual work involved was mainly:
typingsdirectory, which was interfering withopenaitype-hints (such asfrom openai import OpenAI)oaievalset.py.client = OpenAI(api_key=os.environ.get("OPENAI_API_KEY"))line below all the imports.Thus this PR is broken into three parts:
typings(first commit)