Add runtime type checking #22
Draft
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
anishathalye
force-pushed
the
add-beartype
branch
from
39e60b4 to
95a6e29
Compare
anishathalye
force-pushed
the
add-beartype
branch
4 times, most recently
from
ab53920 to
951c5b4
Compare
This patch adds beartype for runtime type checking. This gives us the best of both worlds: we do static type checking of our own library with mypy, and we export our static types, but for clients who do not run static type checking of their own code, runtime type checking in our library can help them catch bugs earlier. `tests/test_codex_tool.py::test_bad_argument_type` serves as an example: this fails at initialization time of `CodexTool`, whereas without runtime type checking, this would fail later (e.g., when the user calls the `query` method on the object). Because we're performing runtime type checking, some of the imports that were behind `if TYPE_CHECKING` flags have to be moved to runtime. This patch updates the linter config to allow imports that are only used for type checking. This patch also switches to consistent `from __future__ import annotations` everywhere, stops using type hints deprecated by PEP 585 by using `beartype.typing` instead, and updates the linter config accordingly. This patch also updates the CI config to run static type checking for all supported Python versions. beartype relies on `isinstance` for runtime type checks, which needs to be taken into account when using mocks by overriding the `__class__` attribute. This patch updates the tests accordingly.
anishathalye
force-pushed
the
add-beartype
branch
from
951c5b4 to
7f1eae0
Compare
|
This patch does add some complexity to development. We should consider whether the benefits outweigh the costs. Happy to discuss synchronously if easier. This patch was motivated by https://github.com/cleanlab/codex/issues/255. |
|
Discussed with Angela: plan is to add custom linters (e.g., using semgrep) so developers don't need to remember all the rules introduced in this PR, which seems like it could have been error-prone. |
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.
This patch adds beartype for runtime type checking. This gives us the best of both worlds: we do static type checking of our own library with mypy, and we export our static types, but for clients who do not run static type checking of their own code, runtime type checking in our library can help them catch bugs earlier.
tests/test_codex_tool.py::test_bad_argument_typeserves as an example: this fails at initialization time ofCodexTool, whereas without runtime type checking, this would fail later (e.g., when the user calls thequerymethod on the object).Because we're performing runtime type checking, some of the imports that were behind
if TYPE_CHECKINGflags have to be moved to runtime. This patch updates the linter config to allow imports that are only used for type checking.This patch also switches to consistent
from __future__ import annotationseverywhere, stops using type hints deprecated by PEP 585 by usingbeartype.typinginstead, and updates the linter config accordingly. This patch also updates the CI config to run static type checking for all supported Python versions.beartype relies on
isinstancefor runtime type checks, which needs to be taken into account when using mocks by overriding the__class__attribute. This patch updates the tests accordingly.