style(core): fix mypy no-any-return violations#34204
style(core): fix mypy no-any-return violations#34204Mason Daugherty (mdrxy) merged 8 commits intolangchain-ai:masterfrom
Conversation
dcb7159 to
3a71eb2
Compare
| # Check if a runtime streaming flag has been passed in. | ||
| if "stream" in kwargs: | ||
| return kwargs["stream"] | ||
| return bool(kwargs["stream"]) |
There was a problem hiding this comment.
calling bool() here doesn't change the behavior as _should_stream is always called in a if condition.
|
|
||
| _SingleMessage = BaseMessage | str | dict[str, Any] | ||
| _T = TypeVar("_T", bound=_SingleMessage) | ||
| # A sequence of _SingleMessage that is NOT a bare str |
There was a problem hiding this comment.
This is needed because otherwise str matches Sequence[str]
CodSpeed Performance ReportMerging #34204 will not alter performanceComparing
|
cce7d38 to
ca9ecff
Compare
|
|
||
| prompt = ChatPromptTemplate(messages=[self]) | ||
| return prompt + other | ||
| return prompt.__add__(other) |
There was a problem hiding this comment.
mypy doesn't figure the + operator...
| try: | ||
| while True: | ||
| chunk = await coro_with_context(py_anext(iterator), context) | ||
| chunk = await coro_with_context(anext(iterator), context) |
There was a problem hiding this comment.
we now use python 3.10+ so py_anext is not needed anymore
31cf767 to
1bac6c8
Compare
| Args: | ||
| coro: The coroutine to await. | ||
| context: The context to use. | ||
| create_task: Whether to create a task. |
There was a problem hiding this comment.
Removing create_task is technically breaking (even though it wasn't exposed in __init__.py)
There was a problem hiding this comment.
Let's add a deprecation warning if create_task=True is passed.
|
|
||
|
|
||
| def get_from_dict_or_env( | ||
| data: dict[str, Any], |
There was a problem hiding this comment.
I think values is dict[str, Any] that can contain floats, objects, etc., not dict[str, str].
There was a problem hiding this comment.
From its signature get_from_dict_or_env must return str and we return the value of the data dict.
So data should be dict[str, str] or else get_from_dict_or_env should be changed to return Any.
There was a problem hiding this comment.
Changing to dict[str, str] would be a breaking change. I understand the type inconsistency concern. If we want stricter correctness, we could add a runtime check, but I'd lean toward keeping dict[str, Any] for compatibility for now.
There was a problem hiding this comment.
I changed to call str(data[key]) so the signature doesn't break. Can you take a look again ?
|
|
||
| def _parse_json( | ||
| json_str: str, *, parser: Callable[[str], Any] = parse_partial_json | ||
| json_str: str, *, parser: Callable[[str], dict[str, Any]] = parse_partial_json |
There was a problem hiding this comment.
parse_partial_json is typed to return Any, not dict[str, Any]
There was a problem hiding this comment.
Yes but mypy misses it and doesn't complain, so there's no need to cast.
There was a problem hiding this comment.
The other possibility would be to make _parse_json/parse_json_markdown return Any which would be closer to the truth. And have callers cast or check the type the result. There are callers of parse_json_markdown in core, langchain-classic and community.
It seems some callers already expect parse_json_markdown to return something different than a dict and have isinstance checks (eg. JSONAgentOutputParser.parse)
LMK what you prefer.
There was a problem hiding this comment.
Mypy not catching something doesn't make the types correct. I think making both return Any is a more honest approach.
There was a problem hiding this comment.
done
|
^outside of the above, looks good! |
6957e92 to
597ad79
Compare
597ad79 to
8826c21
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses mypy's no-any-return violations by fixing type annotations and adding explicit casts where necessary. The main goal is to enable stricter mypy checking by removing the warn_return_any = false configuration setting.
Key Changes:
- Added explicit type parameters to generic classes (e.g.,
RunnableLambda[int, int]) - Added
caststatements to satisfy mypy where type inference fails - Fixed return types and improved type annotations throughout the codebase
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| libs/core/pyproject.toml | Removed warn_return_any = false to enable stricter type checking |
| libs/core/tests/unit_tests/test_tools.py | Fixed test functions to return properly typed values and convert integers to strings |
| libs/core/tests/unit_tests/runnables/test_utils.py | Added type parameters to RunnableLambda instances and fixed return type casting |
| libs/core/tests/unit_tests/runnables/test_tracing_interops.py | Added type parameters to RunnableLambda and added explicit casting for dictionary access |
| libs/core/tests/unit_tests/runnables/test_runnable_events_v2.py | Added type parameters to RunnableLambda instances |
| libs/core/tests/unit_tests/runnables/test_runnable_events_v1.py | Added type parameters to RunnableLambda instances |
| libs/core/tests/unit_tests/runnables/test_runnable.py | Added explicit integer casting for kwargs values |
| libs/core/tests/unit_tests/runnables/test_history.py | Fixed assertion statement that was incorrectly chained and updated type hint |
| libs/core/tests/unit_tests/pydantic_utils.py | Replaced is_basemodel_subclass with direct issubclass checks |
| libs/core/tests/unit_tests/prompts/test_structured.py | Replaced is_basemodel_subclass with direct issubclass check |
| libs/core/tests/unit_tests/messages/test_utils.py | Reorganized type ignore comment placement |
| libs/core/langchain_core/vectorstores/utils.py | Added cast for numpy array return value |
| libs/core/langchain_core/utils/*.py | Added casts and improved type annotations across utility modules |
| libs/core/langchain_core/tracers/*.py | Added casts for dictionary access and improved return type annotations |
| libs/core/langchain_core/tools/*.py | Added casts for Pydantic model creation and schema access |
| libs/core/langchain_core/runnables/*.py | Added type parameters and improved type annotations |
| libs/core/langchain_core/prompts/*.py | Added casts, improved type annotations, and added abstract method declaration |
| libs/core/langchain_core/output_parsers/*.py | Added overloads and improved return type handling |
| libs/core/langchain_core/messages/*.py | Enhanced type checking with runtime validation and added casts |
| libs/core/langchain_core/language_models/*.py | Added casts for serialization and improved type annotations |
| libs/core/langchain_core/_api/internal.py | Added cast for module name access |
Comments suppressed due to low confidence (1)
libs/core/tests/unit_tests/runnables/test_history.py:1
- The assertion on line 100 is now separate from the list comprehension on line 98, but
outputwill be a list containing the boolean result of the comparison that was previously on line 99. The code should assign the result of the list comprehension tooutputbefore asserting. Remove the comparison from inside the list comprehension and keep it only in the assert statement.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return StringPromptValue(text=await self.aformat(**kwargs)) | ||
|
|
||
| @override | ||
| @abstractmethod |
There was a problem hiding this comment.
Adding an abstract method declaration to a base class that likely already has concrete implementations in subclasses could break existing code. If format is already implemented in this class elsewhere (not visible in the diff), this abstract declaration is incorrect. Verify that this method is not already implemented in StringPromptTemplate or ensure all subclasses properly override it.
| @abstractmethod |
There was a problem hiding this comment.
Hi Copilot, you missed that this is an override of the parent method which is already abstract so it doesn't break anything 😉.
456c0fb to
5d5e90f
Compare
a92c032
into
langchain-ai:master
* FIxed where possible * Used `cast` when not possible to fix --------- Co-authored-by: Mason Daugherty <github@mdrxy.com> Co-authored-by: Mason Daugherty <mason@langchain.dev>
castwhen not possible to fix