Develop#77
Conversation
Implement first storage layer
Refactor timeseriesservice
Sync/develop with main
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e563e771f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import pandas as pd | ||
| from matplotlib.backends.backend_tkagg import FigureCanvasTkAgg | ||
| from argus.analytics.charts.trend_chart import create_trendchart | ||
| from argus.services.market_data_service import prepare_trend_analysis |
There was a problem hiding this comment.
market_data_service only exports get_market_data; the prepare_trend_analysis function added in this commit lives in argus.services.trend_analysis_service. Importing argus.gui.app now raises ImportError, and main.py imports this module at startup, so the GUI cannot launch in any environment until this import is corrected.
Useful? React with 👍 / 👎.
| df = pd.DataFrame() | ||
| fig = prepare_trend_analysis(df) |
There was a problem hiding this comment.
After the import is corrected, clicking the Trend button still calls the new prepare_trend_analysis API with only an empty DataFrame, but the service signature is prepare_trend_analysis(db, request). This path will raise TypeError instead of fetching market data, so construct and pass the database path plus a MarketDataRequest rather than the placeholder DataFrame.
Useful? React with 👍 / 👎.
| df = response.bars.copy() | ||
| df = add_daily_percentage_change(df) |
There was a problem hiding this comment.
Successful get_market_data responses now carry the normalized price-bar schema (timestamp, close, etc.), but the existing metric/chart helpers still read df['rate'] and later df['date']. Any successful trend analysis therefore raises KeyError before charting; map close/timestamp to the expected names or update the metrics and chart to consume the new schema before calling them.
Useful? React with 👍 / 👎.
| WHERE data_sources.name = ? | ||
| AND instruments.symbol = ? | ||
| AND price_bars.timestamp BETWEEN ? AND ? |
There was a problem hiding this comment.
The cache lookup filters only source, instrument, and date range, while get_market_data treats any non-empty result as a storage hit. If daily bars were cached and a later request asks for 1h or 15m over the same dates, this query returns the daily rows and skips the client fetch, producing results at the wrong granularity; persist and filter the requested timeframe or bypass the cache when it differs.
Useful? React with 👍 / 👎.
1 participant
What changed?
Related issue
Tests
pytestChecklist
Notes