[mrconway]

Summary

First step of the alphapy-finance split (see plan: `docs/PLAN-split.md` not yet committed; tracked at `~/.claude/plans/okay-we-have-a-snappy-clock.md`).

Removes the only module-level finance imports in the ML core: `yfinance` and `AlpacaDataSource/PolygonDataSource` from `alphapy.data_sources`. They are now imported inside the three functions that use them.

This means `import alphapy` no longer requires yfinance, alpaca-py, or polygon-api-client. Required for the upcoming split where those deps live in alphapy-finance downstream.

Test plan

• `pytest tests/test_imports.py tests/test_utilities.py tests/test_version.py -v` — 18 passed, 1 skipped
• Confirm CI green before merge to develop
• After merge, the next PR (alphapy-finance migration) can begin

[claude]

Claude finished @mrconway's task —— View job

---

PR Review: Lazy-import finance deps in alphapy/data.py

Todo List:

• Read and analyze the changed file (alphapy/data.py)
• Evaluate the lazy import implementation
• Check for code quality and best practices
• Assess potential bugs or issues
• Review performance considerations
• Check security concerns
• Evaluate test coverage implications
• Provide comprehensive review feedback

Review Summary

This PR successfully implements lazy imports for finance dependencies (yfinance, AlpacaDataSource, PolygonDataSource) by moving them from module-level imports to function-level imports. This is a solid preparation step for the upcoming alphapy-finance split.

✅ Strengths

Code Quality & Best Practices

• Clean implementation: The lazy imports are placed exactly where they're needed, just before usage
• Minimal changes: Only 3 lines added, 2 removed - very surgical approach
• Consistent patterns: All three functions follow the same lazy import pattern
• Clear purpose: Well-aligned with the split preparation goals

Performance Considerations

• Faster module loading: import alphapy no longer requires heavy finance dependencies
• On-demand loading: Finance libraries only loaded when specific data sources are used
• Memory efficiency: Dependencies not loaded unless actively needed

⚠️ Areas for Consideration

Import Placement

• Function-level imports: While functional, imports inside functions can impact code readability. However, this is acceptable given the temporary nature for the split preparation.
• Duplicate pandas import: In get_yahoo_data(), pandas is imported twice - once at function level (line 428) and once for yfinance usage (line 429). This is harmless but slightly redundant.

Code Organization

• Documentation gap: Consider adding a brief comment explaining why these are lazy imports (e.g., # Lazy import for alphapy-finance split preparation)

🔍 Technical Analysis

Security

• ✅ No security concerns: The changes don't introduce any security risks
• ✅ Same import sources: Still importing from the same trusted modules

Error Handling

• ✅ Import errors: Will be naturally caught and propagated if dependencies are missing
• ✅ Backwards compatibility: Existing functionality remains unchanged

Test Coverage

The mentioned test results (18 passed, 1 skipped) indicate good coverage. The lazy imports shouldn't affect test behavior since the same code paths are executed.

📝 Minor Suggestions

1. Add explanatory comments (alphapy/data.py:355,429,587):

   # Lazy import for alphapy-finance split preparation
   from alphapy.data_sources import PolygonDataSource

2. Consider consolidating pandas imports in get_yahoo_data() (not critical):

   # yfinance returns pandas, convert to polars
   import pandas as pd
   import yfinance as yf  # Could move up with pandas import

✅ Overall Assessment

This is a well-executed, low-risk change that achieves its goal effectively. The lazy import approach is appropriate for this transition phase, and the implementation is clean and consistent.

Recommendation: APPROVE ✅

The PR successfully removes module-level finance dependencies while maintaining all functionality. This sets up the codebase well for the upcoming alphapy-finance split.

---