Skip to content

Conversation

@TexasCoding
Copy link
Owner

Summary

  • Bumped version to v3.1.12
  • Enhanced the 01_events_with_on.py example with CSV export and interactive charting capabilities
  • Updated all documentation to reflect the new version

Changes

Enhanced Real-time Data Example

  • Added CSV export functionality for bar data
  • Interactive candlestick chart generation using Plotly
  • Automatic prompt after 10 bars to export data and generate charts
  • Non-blocking user input handling for CSV export confirmation
  • Proper bar counting and display formatting
  • Chart opens automatically in browser when generated

Documentation Updates

  • Updated CHANGELOG.md with v3.1.12 release notes
  • Updated CLAUDE.md with recent changes section
  • Version bumped to 3.1.12 in pyproject.toml and init.py

Test Plan

  • Version numbers updated consistently across all files
  • Example script runs without errors
  • CSV export functionality works correctly
  • Chart generation works when Plotly is installed
  • Example gracefully handles missing Plotly dependency
  • Non-blocking input prompt works as expected

🤖 Generated with Claude Code

TexasCoding and others added 2 commits August 14, 2025 05:27
- Fix timezone handling in get_bars to use America/Chicago instead of UTC
- Add last_bar_times tracking when loading historical data
- Add warning when historical data has significant gaps
- Improve managed_trade cleanup to preserve protective orders
- Fix linting issues in realtime_data_manager

The historical data API has an inherent delay (typically 1+ hour for short
timeframes), which is a platform limitation. The system now properly handles
this gap and warns users when it occurs.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
- Enhanced 01_events_with_on.py example with CSV export and charting
- Added Plotly-based candlestick chart generation
- Interactive prompts for data export after 10 bars
- Non-blocking user input handling
- Better price formatting and visual indicators
- Updated version to 3.1.12 across all files
- Updated CHANGELOG and documentation

🤖 Generated with Claude Code (https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@claude
Copy link

claude bot commented Aug 15, 2025

Claude finished @TexasCoding's task —— View job


PR Review - v3.1.12 Release

Review Tasks:

  • Read and analyze the main enhanced example file (01_events_with_on.py)
  • Review the new risk manager demo file
  • Check version consistency across all modified files
  • Review code quality, best practices, and potential issues
  • Assess security and performance considerations
  • Provide comprehensive review feedback in GitHub comment

📋 Overall Assessment

This PR introduces significant enhancements to the real-time data example and adds a comprehensive risk manager demo. The changes are well-implemented with good attention to error handling and user experience.

✅ Strengths

Enhanced Example File (01_events_with_on.py)

  • Excellent error handling: Graceful degradation when Plotly is unavailable
  • Non-blocking user input: Smart async implementation using loop.run_in_executor()
  • Resource management: Proper cleanup with async context managers
  • User experience: Clear prompts, visual indicators, and automatic browser opening
  • Code organization: Well-structured functions with clear separation of concerns

Risk Manager Demo (16_risk_manager_live_demo.py)

  • Comprehensive coverage: Demonstrates all risk management features effectively
  • Safety features: demo_trades_enabled flag to prevent accidental trades
  • Robust error handling: Multiple retry attempts for critical operations like stop adjustments
  • Educational value: Extensive logging and explanations throughout
  • Production-ready patterns: Uses proper async context managers and dependency injection

Version Management

  • Consistent versioning: Version 3.1.12 properly updated across all relevant files
  • Complete changelog: Well-documented changes with clear categorization

⚠️ Areas for Improvement

Code Quality Issues

  1. Hardcoded timeout values in 01_events_with_on.py:188:

    response = await asyncio.wait_for(future, timeout=10.0)

    Consider making this configurable or at least documenting why 10 seconds was chosen.

  2. Potential memory leak in 01_events_with_on.py:274:

    asyncio.create_task(prompt_for_csv_export(suite, TIMEFRAME))  # noqa: RUF006

    The task is created but never awaited or stored for cleanup. Consider storing tasks for proper cleanup.

  3. Exception swallowing in 01_events_with_on.py:112:

    except Exception:
        pass  # Silently fail if browser can't be opened

    While appropriate here, consider logging at debug level for troubleshooting.

  4. Type safety concerns in 16_risk_manager_live_demo.py:209-230:
    The mock Order creation uses multiple type casts which could mask type issues. Consider using a factory method or builder pattern.

Security Considerations

  1. File path handling in 01_events_with_on.py:140:

    filename = f"bars_export_{suite.instrument.name}_{timeframe}_{timestamp}.csv"

    No validation of suite.instrument.name - could potentially contain path traversal characters.

  2. Real trading warnings: The risk demo appropriately warns about real positions but could be more prominent (consider ASCII art warning).

Performance Considerations

  1. Chart generation blocking: The create_candlestick_chart() function processes entire DataFrames synchronously, which could block for large datasets.

  2. Inefficient DataFrame iteration in 01_events_with_on.py:256:

    for row in last_bars.tail(6).iter_rows(named=True):

    Consider using vectorized operations for better performance with larger datasets.

🔧 Specific Recommendations

Priority: High

  1. Add input validation for file paths to prevent directory traversal
  2. Store and cleanup async tasks to prevent resource leaks
  3. Add configuration options for timeouts and limits

Priority: Medium

  1. Extract magic numbers to constants (10 bars, 10 second timeout, etc.)
  2. Add debug logging for browser opening failures
  3. Consider async chart generation for large datasets

Priority: Low

  1. Add docstring examples for the new CSV export functionality
  2. Consider adding progress indicators for long-running operations

🏆 Test Coverage Assessment

The PR lacks explicit tests for the new functionality. Consider adding:

  • Unit tests for CSV export functionality
  • Integration tests for chart generation (with mocked Plotly)
  • Tests for the async user input handling

📊 Final Verdict

Recommendation: APPROVE with minor follow-up

This is a solid enhancement that significantly improves the user experience and educational value of the examples. The code quality is good overall with only minor issues that should be addressed in future PRs rather than blocking this release.

The new features are well-implemented, properly documented, and follow the project's async-first architecture. The risk manager demo is particularly valuable for users learning the platform.

@TexasCoding TexasCoding merged commit 7975f33 into main Aug 15, 2025
4 checks passed
@TexasCoding TexasCoding deleted the examples branch August 15, 2025 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants