Lista di cryptovalute #36

2025-10-19T18:39:33+02:00

Berack96 commented

2025-10-19 18:39:33 +02:00

(Migrated from github.com)

This pull request introduces a new tool for retrieving cryptocurrency symbols, refactors how market tools are initialized, and removes the currency configuration from the API settings. These changes enhance the flexibility and maintainability of the codebase, particularly in how market data is accessed and managed.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2025-10-21 13:23:20 +02:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

This pull request introduces a new cryptocurrency symbols lookup tool and refactors the market API initialization by removing the hardcoded currency configuration. The main purpose is to enhance flexibility in handling cryptocurrency symbols and improve maintainability by simplifying the currency handling logic.

Key changes:

Adds a new CryptoSymbolsTools class that fetches and caches cryptocurrency symbols from Yahoo Finance
Removes the currency parameter from MarketAPIsTool initialization and the APIConfig class
Refactors symbol formatting logic in market wrappers to consistently extract the base asset before appending the currency

Reviewed Changes

Copilot reviewed 11 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/tools/test_crypto_symbols.py	Adds comprehensive test coverage for the new CryptoSymbolsTools functionality
src/app/configs.py	Removes the currency field from APIConfig
src/app/api/tools/symbols_tool.py	Implements the new CryptoSymbolsTools class for fetching and searching cryptocurrency symbols
src/app/api/tools/market_tool.py	Removes currency parameter from MarketAPIsTool initialization
src/app/api/tools/init.py	Exports the new CryptoSymbolsTools class
src/app/api/markets/yfinance.py	Updates symbol formatting to extract base asset before appending currency
src/app/api/markets/coinbase.py	Updates symbol formatting to extract base asset before appending currency
src/app/api/markets/binance.py	Updates symbol formatting to extract base asset before appending currency
src/app/agents/core.py	Integrates CryptoSymbolsTools into the agent team and removes currency parameter from MarketAPIsTool instantiation
pyproject.toml	Adds html5lib dependency for HTML parsing
configs.yaml	Removes currency configuration from API settings

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

## Pull Request Overview This pull request introduces a new cryptocurrency symbols lookup tool and refactors the market API initialization by removing the hardcoded currency configuration. The main purpose is to enhance flexibility in handling cryptocurrency symbols and improve maintainability by simplifying the currency handling logic. Key changes: - Adds a new `CryptoSymbolsTools` class that fetches and caches cryptocurrency symbols from Yahoo Finance - Removes the `currency` parameter from `MarketAPIsTool` initialization and the `APIConfig` class - Refactors symbol formatting logic in market wrappers to consistently extract the base asset before appending the currency ### Reviewed Changes Copilot reviewed 11 out of 13 changed files in this pull request and generated 6 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | tests/tools/test_crypto_symbols.py | Adds comprehensive test coverage for the new CryptoSymbolsTools functionality | | src/app/configs.py | Removes the currency field from APIConfig | | src/app/api/tools/symbols_tool.py | Implements the new CryptoSymbolsTools class for fetching and searching cryptocurrency symbols | | src/app/api/tools/market_tool.py | Removes currency parameter from MarketAPIsTool initialization | | src/app/api/tools/__init__.py | Exports the new CryptoSymbolsTools class | | src/app/api/markets/yfinance.py | Updates symbol formatting to extract base asset before appending currency | | src/app/api/markets/coinbase.py | Updates symbol formatting to extract base asset before appending currency | | src/app/api/markets/binance.py | Updates symbol formatting to extract base asset before appending currency | | src/app/agents/core.py | Integrates CryptoSymbolsTools into the agent team and removes currency parameter from MarketAPIsTool instantiation | | pyproject.toml | Adds html5lib dependency for HTML parsing | | configs.yaml | Removes currency configuration from API settings | </details> --- <sub>**Tip:** Customize your code reviews with copilot-instructions.md. <a href="/Berack96/upo-appAI/new/main/.github?filename=copilot-instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Create the file</a> or <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">learn how to get started</a>.</sub>

src/app/api/markets/binance.py

						
				@@ -57,7 +57,9 @@ class BinanceWrapper(MarketWrapper):

				        """

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-21 13:23:19 +02:00

The str.index() method raises a ValueError when the substring is not found, rather than returning -1. This will cause a runtime error when processing symbols without a hyphen. Use asset_id.find('-') instead, which returns -1 when not found, or handle the ValueError exception.

        i = asset_id.find('-')

The `str.index()` method raises a `ValueError` when the substring is not found, rather than returning -1. This will cause a runtime error when processing symbols without a hyphen. Use `asset_id.find('-')` instead, which returns -1 when not found, or handle the `ValueError` exception. ```suggestion i = asset_id.find('-') ```

src/app/api/markets/coinbase.py

						
				@@ -61,7 +61,9 @@ class CoinBaseWrapper(MarketWrapper):

				        )

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-21 13:23:19 +02:00

The str.index() method raises a ValueError when the substring is not found, rather than returning -1. This will cause a runtime error when processing symbols without a hyphen. Use asset_id.find('-') instead, which returns -1 when not found, or handle the ValueError exception.

        i = asset_id.find('-')

The `str.index()` method raises a `ValueError` when the substring is not found, rather than returning -1. This will cause a runtime error when processing symbols without a hyphen. Use `asset_id.find('-')` instead, which returns -1 when not found, or handle the `ValueError` exception. ```suggestion i = asset_id.find('-') ```

src/app/api/markets/yfinance.py

						
				@@ -47,8 +47,9 @@ class YFinanceWrapper(MarketWrapper):

				        Formatta il simbolo per yfinance.

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-21 13:23:19 +02:00

The str.index() method raises a ValueError when the substring is not found, rather than returning -1. This will cause a runtime error when processing symbols without a hyphen. Use asset_id.find('-') instead, which returns -1 when not found, or handle the ValueError exception.

        i = asset_id.find('-')

The `str.index()` method raises a `ValueError` when the substring is not found, rather than returning -1. This will cause a runtime error when processing symbols without a hyphen. Use `asset_id.find('-')` instead, which returns -1 when not found, or handle the `ValueError` exception. ```suggestion i = asset_id.find('-') ```

src/app/api/tools/symbols_tool.py

						
				@@ -0,0 +1,103 @@

				import os

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-21 13:23:18 +02:00

The return type documentation states 'list[str]' but the actual return type is 'list[tuple[str, str]]'. Update the docstring to reflect the correct return type.

            list[tuple[str, str]]: Lista di tuple (simbolo, nome) che contengono la query.

The return type documentation states 'list[str]' but the actual return type is 'list[tuple[str, str]]'. Update the docstring to reflect the correct return type. ```suggestion list[tuple[str, str]]: Lista di tuple (simbolo, nome) che contengono la query. ```

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-21 13:23:20 +02:00

Corrected grammar in comment from 'It looks like is the max' to 'It looks like this is the max'.

        num_currencies = 250 # It looks like this is the max per page; otherwise, Yahoo returns 26

Corrected grammar in comment from 'It looks like is the max' to 'It looks like this is the max'. ```suggestion num_currencies = 250 # It looks like this is the max per page; otherwise, Yahoo returns 26 ```

src/app/api/tools/symbols_tool.py

						
				@@ -0,0 +20,4 @@

				    def __init__(self, cache_file: str = 'resources/cryptos.csv'):

				        self.cache_file = cache_file

				        self.final_table = pd.read_csv(self.cache_file) if os.path.exists(self.cache_file) else pd.DataFrame() # type: ignore

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-21 13:23:20 +02:00

[nitpick] This line is overly complex with multiple operations. Consider splitting it into separate lines for better readability: check file existence, then read CSV or create empty DataFrame.

        if os.path.exists(self.cache_file):
            self.final_table = pd.read_csv(self.cache_file) # type: ignore
        else:
            self.final_table = pd.DataFrame() # type: ignore

[nitpick] This line is overly complex with multiple operations. Consider splitting it into separate lines for better readability: check file existence, then read CSV or create empty DataFrame. ```suggestion if os.path.exists(self.cache_file): self.final_table = pd.read_csv(self.cache_file) # type: ignore else: self.final_table = pd.DataFrame() # type: ignore ```

Simo93-rgb (Migrated from github.com) approved these changes 2025-10-21 15:58:41 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Berack/upo-app-agents#36

				`@@ -57,7 +57,9 @@ class BinanceWrapper(MarketWrapper):`
				`"""`

				`@@ -61,7 +61,9 @@ class CoinBaseWrapper(MarketWrapper):`
				`)`

				`@@ -47,8 +47,9 @@ class YFinanceWrapper(MarketWrapper):`
				`Formatta il simbolo per yfinance.`

				`@@ -0,0 +1,103 @@`
				`import os`