-
Notifications
You must be signed in to change notification settings - Fork 374
features:Added new tools to the FinancialDatasetAPI #532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,62 @@ | ||||||||||||||||||||
| from typing import Optional, Type | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from langchain_core.callbacks import CallbackManagerForToolRun | ||||||||||||||||||||
| from langchain_core.tools import BaseTool | ||||||||||||||||||||
| from pydantic import BaseModel, Field | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from langchain_community.utilities.financial_datasets import FinancialDatasetsAPIWrapper | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class FinancialMetricsSchema(BaseModel): | ||||||||||||||||||||
| """Input for FinancialMetrics.""" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ticker: str = Field( | ||||||||||||||||||||
| description="The ticker symbol to fetch financial metrics for.", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| period: str = Field( | ||||||||||||||||||||
| description="The period of the financial metrics. " | ||||||||||||||||||||
| "Possible values are: " | ||||||||||||||||||||
| "annual, quarterly, ttm. " | ||||||||||||||||||||
| "Default is 'annual'.", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| limit: int = Field( | ||||||||||||||||||||
| description="The number of financial metrics to return. Default is 10.", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
Comment on lines
+22
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Fix by making the field optional with a default: limit: int = Field(
default=10,
description="The number of financial metrics to return.",
)
Suggested change
Spotted by Graphite Agent |
||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class FinancialMetrics(BaseTool): | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| Tool that gets financial metrics for a given ticker over a given period. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
|
|
||||||||||||||||||||
| mode: str = "get_financial_metrics" | ||||||||||||||||||||
| name: str = "financial_metrics" | ||||||||||||||||||||
| description: str = ( | ||||||||||||||||||||
| "A wrapper around financial datasets's financial metrics API. " | ||||||||||||||||||||
| "This tool is useful for fetching financial metrics for a given ticker." | ||||||||||||||||||||
| "The tool fetches financial metrics for a given ticker over a given period." | ||||||||||||||||||||
| "The period can be annual, quarterly, or trailing twelve months (ttm)." | ||||||||||||||||||||
| "The number of financial metrics to return can also be " | ||||||||||||||||||||
| "specified using the limit parameter." | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| args_schema: Type[FinancialMetricsSchema] = FinancialMetricsSchema | ||||||||||||||||||||
|
|
||||||||||||||||||||
| api_wrapper: FinancialDatasetsAPIWrapper = Field(..., exclude=True) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def __init__(self, api_wrapper: FinancialDatasetsAPIWrapper): | ||||||||||||||||||||
| super().__init__(api_wrapper=api_wrapper) | ||||||||||||||||||||
|
Comment on lines
+46
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The init method violates the 'Use Google-Style Docstrings (with Args section)' rule. This public method lacks a Google-style docstring with an Args section. It should include documentation explaining the api_wrapper parameter. Add a docstring like: '''Initialize the FinancialMetrics tool. Args:
Suggested change
Spotted by Graphite Agent (based on custom rule: Code quality) |
||||||||||||||||||||
|
|
||||||||||||||||||||
| def _run( | ||||||||||||||||||||
| self, | ||||||||||||||||||||
| ticker: str, | ||||||||||||||||||||
| period: str, | ||||||||||||||||||||
| limit: Optional[int], | ||||||||||||||||||||
| run_manager: Optional[CallbackManagerForToolRun] = None, | ||||||||||||||||||||
| ) -> str: | ||||||||||||||||||||
| """Use the financial metrics API tool.""" | ||||||||||||||||||||
| return self.api_wrapper.run( | ||||||||||||||||||||
| mode=self.mode, | ||||||||||||||||||||
| ticker=ticker, | ||||||||||||||||||||
| period=period, | ||||||||||||||||||||
| limit=limit, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,43 @@ | ||||||||||||||||||||
| from typing import Optional, Type | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from langchain_core.callbacks import CallbackManagerForToolRun | ||||||||||||||||||||
| from langchain_core.tools import BaseTool | ||||||||||||||||||||
| from pydantic import BaseModel, Field | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from langchain_community.utilities.financial_datasets import FinancialDatasetsAPIWrapper | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class FinancialSnapshotsSchema(BaseModel): | ||||||||||||||||||||
| """Input for FinancialSnapshots.""" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ticker: str = Field( | ||||||||||||||||||||
| description="The ticker symbol to fetch financial snapshots for.", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class FinancialSnapshots(BaseTool): | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| Tool that gets financial snapshots for a given ticker over a given period. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
|
|
||||||||||||||||||||
| mode: str = "get_financial_snapshots" | ||||||||||||||||||||
| name: str = "financial_snapshots" | ||||||||||||||||||||
| description: str = ( | ||||||||||||||||||||
| "A wrapper around financial datasets's financial snapshots API. " | ||||||||||||||||||||
| "This tool is useful for fetching financial snapshots for a given ticker." | ||||||||||||||||||||
| "The tool fetches financial snapshots for a given ticker." | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| args_schema: Type[FinancialSnapshotsSchema] = FinancialSnapshotsSchema | ||||||||||||||||||||
|
|
||||||||||||||||||||
| api_wrapper: FinancialDatasetsAPIWrapper = Field(..., exclude=True) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def __init__(self, api_wrapper: FinancialDatasetsAPIWrapper): | ||||||||||||||||||||
| super().__init__(api_wrapper=api_wrapper) | ||||||||||||||||||||
|
Comment on lines
+34
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The init method violates the 'Use Google-Style Docstrings (with Args section)' rule. This public method lacks a Google-style docstring with an Args section. It should include documentation explaining the api_wrapper parameter. Add a docstring like: '''Initialize the FinancialSnapshots tool. Args:
Suggested change
Spotted by Graphite Agent (based on custom rule: Code quality) |
||||||||||||||||||||
|
|
||||||||||||||||||||
| def _run( | ||||||||||||||||||||
| self, | ||||||||||||||||||||
| ticker: str, | ||||||||||||||||||||
| run_manager: Optional[CallbackManagerForToolRun] = None, | ||||||||||||||||||||
| ) -> str: | ||||||||||||||||||||
| """Use the financial snapshots API tool.""" | ||||||||||||||||||||
| return self.api_wrapper.run(mode=self.mode, ticker=ticker) | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import os | ||
|
|
||
| from langchain_community.agent_toolkits.financial_datasets.toolkit import ( | ||
| FinancialDatasetsToolkit, | ||
| ) | ||
| from langchain_community.utilities.financial_datasets import FinancialDatasetsAPIWrapper | ||
|
|
||
| api_wrapper = FinancialDatasetsAPIWrapper( | ||
| financial_datasets_api_key=os.environ["FINANCIAL_DATASETS_API_KEY"] | ||
| ) | ||
| toolkit = FinancialDatasetsToolkit(api_wrapper=api_wrapper) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
periodfield in the schema is required (no default value) but the implementation treats it as optional with a default of 'annual'. This mismatch will force users to always provide a period value even though the description says "Default is 'annual'" and the API wrapper useskwargs.get("period", "annual")on line 203. This will cause the tool to fail validation when users expect to rely on the default.Fix by making the field optional with a default:
Spotted by Graphite Agent

Is this helpful? React 👍 or 👎 to let us know.