Add Makefile: one-command setup, test, lint, and format - #7
Open
jrile018 wants to merge 57 commits into
Open
Conversation
need to add more charts and metrics need to theme the webapp and dashboard
docs: 📜 database, dashboard integration docs TCP connections are currently failing
chore: 🧹 ran linting
Added instructions for creating IP-ready results.
Add explicit tickformat parameter to ensure dates are displayed in yyyy-mm-dd format instead of milliseconds since epoch. This fixes the issue where equity curve graphs and other time-series charts were showing non-human-legible date encodings.
Added date_format='%%Y-%%m-%%d' parameter to all pandas to_csv() calls: - Line 449: timeseries CSV export - Line 498: equity curve CSV export - Line 508: drawdown CSV export - Line 520: risk metrics CSV export When pandas exports datetime indices without date_format, it converts dates to millisecond timestamps. This was the root cause of all charts displaying non-human-legible datetime values instead of yyyy-mm-dd format.
Also add date_format='%%Y-%%m-%%d' to the portfolio_value CSV export (line 460) to ensure consistent datetime formatting across all CSV exports.
Major improvements to ensure datetime is properly handled: 1. Enhanced _load_timeseries_csv(): - Added smarter date column detection (Date, DateTime, Timestamp) - Use infer_datetime_format=True for faster parsing - Check 90% conversion success ratio before setting as index - Ensure index is always datetime64[ns] 2. Added _ensure_datetime_index() helper: - Converts any index to datetime if not already - Applied to all chart functions before plotting 3. Enhanced _make_fig_base(): - Added explicit type='date' to x-axis configuration - Ensures Plotly treats x-axis as date axis, not numeric 4. Applied datetime enforcement to: - _save_line_chart() - _save_comparison_chart() - _save_drawdown_chart() This fix ensures that datetime values are preserved throughout the entire pipeline: CSV loading -> DataFrame index -> Plotly rendering
Issues fixed: 1. Removed deprecated infer_datetime_format=True parameter from pd.to_datetime() - This parameter is deprecated in recent pandas versions - Can cause issues with datetime parsing 2. Removed type='date' from Plotly x-axis configuration - Plotly handles datetime axis type automatically with datetime data - Explicit type='date' can interfere with pandas Timestamp objects - tickformat='%%Y-%%m-%%d' is sufficient for date formatting 3. Made _ensure_datetime_index() more conservative - Only converts if index looks like datetime strings (has - / or length >= 8) - Only applies conversion if >90% of values convert successfully - Returns data unchanged if already datetime (no double conversion) These minimal changes preserve datetime formatting while restoring data visibility in charts.
KEY INSIGHT: The problem wasn't binary - it required converting datetime data at the RIGHT point in the pipeline. Solution: 1. Keep datetime index during CSV loading and data processing - Ensures proper datetime operations on the data 2. Convert datetime index to ISO format strings BEFORE passing to Plotly - New helper: _convert_index_to_iso_strings() - Converts DatetimeIndex to 'YYYY-MM-DD' format strings - Applied in all three chart functions after _ensure_datetime_index() 3. Use Plotly's type='date' with these ISO strings - Plotly recognizes the strings as dates - Applies tickformat='%%Y-%%m-%%d' for consistent formatting - Data renders properly because Plotly gets string data it understands 4. Simplified Plotly config - Removed problematic rangebreaks and rangeselector - Focus on core datetime formatting: type='date' + tickformat Benefits: ✓ Datetime properly formatted (YYYY-MM-DD) on chart axes ✓ Data visible and rendered (no NaT explosion) ✓ Hover templates show dates without format specifiers (already formatted) ✓ Command remains same: algosystem ip strategy.csv -b sp500
Paired with branch protection requiring 2 approvals + "require review from Code Owners", this enforces the policy that every PR into main is approved by one repo admin plus one other person. GitHub has no native "must include an admin" setting, so the admin list is expressed here and enforced via the code-owner review requirement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
make install # poetry install make test # pytest make test-cov # pytest with coverage report make lint # black + isort + ruff, read-only make format # black + isort, auto-fix make typecheck # mypy Targets: install, test, test-cov, lint, format, typecheck, docs, clean, help. `make help` is the default goal and self-documents from the target comments. Details: - lint is read-only (--check / --check-only) and format is the auto-fix counterpart, so the two are never confused -- lint can be wired into CI without it rewriting files. - Every tool used is already a dev dependency (black, isort, ruff, mypy, pytest-cov); no new dependencies are introduced. - docs guards on docs/conf.py existing and prints a clear message if not, because the Sphinx config lands in a separate open PR. Without the guard it would fail with a confusing sphinx error on this branch. Verified in an ubuntu:22.04 container (make is unavailable on the authoring machine): parses under GNU Make 4.3 and help renders correctly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jrile018
requested review from
colerottenberg,
dominickdupuy,
patrickbott,
raohemdutt,
roshan-h-shah and
ry2009
as code owners
July 25, 2026 22:59
This branch forked before the DDD restructure (#24), so merging it as-is resurrected 78 files that main deliberately deleted -- the whole old dashboard, analysis/ and api.py. None of that is what the PR is for. Resolved by taking main's version of everything and keeping only the Makefile, then aligning it with what main actually has now: - `lint` runs ruff over tests as well as the package, and adds `lint-imports`, so `make lint` matches what test.yml enforces on a PR rather than a weaker subset that passes locally and fails in CI. - Kept a `check` target -- main's Makefile had exactly one target by that name, and anything calling it should keep working. It is now `check: lint test`. Verified against main rather than assumed: docs/conf.py and .importlinter both exist, so the `docs` guard and `lint-imports` are not aspirational, and the help target's parser lists all ten targets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of a cross-repo pass adding a consistent
make installto all four AlgoGators repos.Usage
make helpis the default goal — just runmake.Notes on the implementation
lintandformatare deliberately separate.lintis read-only (--check/--check-only),formatis the auto-fix counterpart. That meanslintcan be wired into CI without it silently rewriting files.docsis guarded. The Sphinx config (docs/conf.py) lands in a separate open PR, so this target checks for it and prints a clear message if absent. Without the guard you'd get a confusing sphinx error on this branch.Verification
makeisn't available on the authoring machine, so this was tested in anubuntu:22.04container: parses under GNU Make 4.3 andmake helprenders correctly.