Refactor timeseriesservice#75
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 469c45833d
ℹ️ 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.
Import the trend service from the module that defines it
Launching the GUI now fails during import because market_data_service only defines get_market_data; the new prepare_trend_analysis function is defined in argus.services.trend_analysis_service. In any environment with the declared dependencies installed, importing argus.gui.app or argus.main raises ImportError before the app window can start.
Useful? React with 👍 / 👎.
| return response.message | ||
|
|
||
| df = response.bars.copy() | ||
| df = add_daily_percentage_change(df) |
There was a problem hiding this comment.
Convert price bars before running trend metrics
When get_market_data succeeds, response.bars uses the new price-bar schema (timestamp/close), but the existing trend metric functions still index rate and later date. A normal successful trend request therefore raises KeyError: 'rate' here instead of returning a figure; this was masked because the added trend-analysis tests are inside a module-level string.
Useful? React with 👍 / 👎.
| FOREIGN KEY (source_id) REFERENCES data_sources (id), | ||
| FOREIGN KEY (instrument_id) REFERENCES instruments (id), | ||
| UNIQUE (source_id, instrument_id, timestamp, timeframe) | ||
| UNIQUE (source_id, instrument_id, timestamp,close) |
There was a problem hiding this comment.
Keep timeframe in stored price-bar identity
The request model still carries timeframe, and get_market_data checks storage before calling yfinance, but the table no longer stores or keys by timeframe. After caching daily bars for an instrument/date range, a later hourly request for the same source/instrument/dates will be treated as a storage hit and return the daily rows instead of fetching hourly data.
Useful? React with 👍 / 👎.
What changed?
time_series_serviceinto two new services:market_data_serviceandtrend_analysis_servicetrend_analysis_servicecalls analytics not vise versaRelated issue
Closes #70
Tests
pytestChecklist
Notes