features:Added new tools to the FinancialDatasetAPI#532
features:Added new tools to the FinancialDatasetAPI#532Jonathan Reynold Gomes (devgomesai) wants to merge 4 commits intolangchain-ai:mainfrom
Conversation
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| def __init__(self, api_wrapper: FinancialDatasetsAPIWrapper): | ||
| super().__init__(api_wrapper=api_wrapper) |
There was a problem hiding this comment.
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:
api_wrapper: The FinancialDatasetsAPIWrapper instance for API calls.'''
| def __init__(self, api_wrapper: FinancialDatasetsAPIWrapper): | |
| super().__init__(api_wrapper=api_wrapper) | |
| def __init__(self, api_wrapper: FinancialDatasetsAPIWrapper): | |
| """Initialize the FinancialMetrics tool. | |
| Args: | |
| api_wrapper: The FinancialDatasetsAPIWrapper instance for API calls. | |
| """ | |
| super().__init__(api_wrapper=api_wrapper) |
Spotted by Graphite Agent (based on custom rule: Code quality)
Is this helpful? React 👍 or 👎 to let us know.
| def __init__(self, api_wrapper: FinancialDatasetsAPIWrapper): | ||
| super().__init__(api_wrapper=api_wrapper) |
There was a problem hiding this comment.
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:
api_wrapper: The FinancialDatasetsAPIWrapper instance for API calls.'''
| def __init__(self, api_wrapper: FinancialDatasetsAPIWrapper): | |
| super().__init__(api_wrapper=api_wrapper) | |
| def __init__(self, api_wrapper: FinancialDatasetsAPIWrapper): | |
| """Initialize the FinancialSnapshots tool. | |
| Args: | |
| api_wrapper: The FinancialDatasetsAPIWrapper instance for API calls. | |
| """ | |
| super().__init__(api_wrapper=api_wrapper) |
Spotted by Graphite Agent (based on custom rule: Code quality)
Is this helpful? React 👍 or 👎 to let us know.
| period: str = Field( | ||
| description="The period of the financial metrics. " | ||
| "Possible values are: " | ||
| "annual, quarterly, ttm. " | ||
| "Default is 'annual'.", | ||
| ) |
There was a problem hiding this comment.
The period field 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 uses kwargs.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:
period: str = Field(
default="annual",
description="The period of the financial metrics. "
"Possible values are: "
"annual, quarterly, ttm.",
)| period: str = Field( | |
| description="The period of the financial metrics. " | |
| "Possible values are: " | |
| "annual, quarterly, ttm. " | |
| "Default is 'annual'.", | |
| ) | |
| period: str = Field( | |
| default="annual", | |
| description="The period of the financial metrics. " | |
| "Possible values are: " | |
| "annual, quarterly, ttm.", | |
| ) |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
| limit: int = Field( | ||
| description="The number of financial metrics to return. Default is 10.", | ||
| ) |
There was a problem hiding this comment.
The limit field in the schema is required (no default value) but the implementation treats it as optional with a default of 10. The _run method signature shows limit: Optional[int] on line 53, and the API wrapper uses kwargs.get("limit", 10) on line 204. This mismatch will force users to always provide a limit value even though the description says "Default is 10." This will cause validation errors when users expect to rely on the default.
Fix by making the field optional with a default:
limit: int = Field(
default=10,
description="The number of financial metrics to return.",
)| limit: int = Field( | |
| description="The number of financial metrics to return. Default is 10.", | |
| ) | |
| limit: int = Field( | |
| default=10, | |
| description="The number of financial metrics to return.", | |
| ) |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
|
Linked to following Issue: #531 |
|
Jonathan Reynold Gomes (@devgomesai) it would be really good for the community if you can post the Docs that you have followed. |
|
2 participants
Description
This PR extends the FinancialDatasetsToolkit by adding two new tools:
Both tools are initialized with the existing api_wrapper, ensuring consistent authentication, configuration, and reuse across the toolkit.
These additions make commonly used Financial Datasets endpoints available as first-class LangChain tools.
Issue
N/A
Dependencies
None.
No new dependencies were added and no existing dependency versions were modified.
Lint and test
make format
make lint
make test
All relevant checks have been run locally.
Additional notes