Some parts of this code were including typehints, but we haven't been running any checks to verify the correctness of those annotations. Static typechecking helps to improve overall quality and catch mistakes.
Solution
Add a dev dependency on mypy (site) and packages with types for third-party dependencies.
Make adjustments throughout the code to satify the typechecker
Add steps in CI to run mypy checks automatically on every change.
For now, exclude generated code in pinecone/core because it has a bunch of issues but is not easy to change.
Summary of code changes:
Everywhere that has a default value of None needs to be wrapped in the Optional type from the typings package. Correct: def my_method(foo: Optional[str] = None). Incorrect: def my_method(foo: str = None)
Refactored config-related stuff a bit because having config classes with inner _config attributes seemed like a smell and also made typing config-related stuff more challening. Ended up converting what was previously the Config class into a ConfigBuilder and the actual config object is now just a NamedTuple which is a container for immutable data.
Mypy doesn't like when you overwrite variables in a way that changes the type. So some places required me to introduce an additional intermediate variable to satisfy the type checker.
Mypy got mad about having a Pinecone class in both pinecone and pinecone.grpc. So I renamed the GRPC one to PineconeGRPC.
Problem
Some parts of this code were including typehints, but we haven't been running any checks to verify the correctness of those annotations. Static typechecking helps to improve overall quality and catch mistakes.
Solution
mypy
(site) and packages with types for third-party dependencies.pinecone/core
because it has a bunch of issues but is not easy to change.Summary of code changes:
None
needs to be wrapped in theOptional
type from thetypings
package. Correct:def my_method(foo: Optional[str] = None)
. Incorrect:def my_method(foo: str = None)
_config
attributes seemed like a smell and also made typing config-related stuff more challening. Ended up converting what was previously theConfig
class into aConfigBuilder
and the actual config object is now just aNamedTuple
which is a container for immutable data.Pinecone
class in bothpinecone
andpinecone.grpc
. So I renamed the GRPC one toPineconeGRPC
.Type of Change
Test Plan
See new step running in CI.