Release v1.1.0 #79
Merged
Release v1.1.0 #79
Conversation
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
Problem: - Simple commands like `keecas --version` took 5.8 seconds - CLI entry point loaded heavy dependencies (SymPy, Pint) unnecessarily - Every CLI invocation imported keecas package which triggered __init__.py Solution: - Created lightweight wrapper `keecas_cli.py` outside keecas package - Fast paths for common commands that don't need heavy dependencies: * --version/-V: Direct metadata access (5.8s -> 0.17s, ~34x faster) * --help/-h: Quick help without argparse (2.5s -> 0.14s, ~18x faster) * No args: Fast error message (2.5s -> 0.14s) - Commands needing full functionality delegate to keecas.cli Performance improvements: - `keecas --version`: 5.834s -> 0.172s (~34x faster) - `keecas --help`: 2.529s -> 0.142s (~18x faster) - Other commands (config, edit) still work, load keecas when needed Changes: - Add src/keecas_cli.py: Lightweight CLI entry point with lazy imports - Update pyproject.toml: Change entry point from keecas.cli:main to keecas_cli:main
…-08ew6 perf: optimize CLI startup time by 34x for simple commands
The subprocess.Popen() call was using PIPE for stdout/stderr but never consuming the output. This causes the process to hang when the OS pipe buffers fill up after extended Jupyter usage. Solution: Let Jupyter output go directly to the terminal (stdout=None, stderr=None) which prevents buffer deadlock and allows users to see Jupyter logs. Fixes #71 Co-authored-by: kompre <kompre@users.noreply.github.com>
Co-authored-by: kompre <kompre@users.noreply.github.com>
fix: prevent Jupyter kernel hang by removing stdout/stderr pipes
chore: bump version to 1.0.5b1 for test release
…ution The lightweight CLI entry point (keecas_cli.py) was located outside the keecas package at src/keecas_cli.py, which prevented it from being included in the built package. This caused "ModuleNotFoundError: No module named 'keecas'" when running CLI commands after installation. Changes: - Moved src/keecas_cli.py -> src/keecas/__main__.py (inside package) - Updated pyproject.toml entry point: keecas_cli:main -> keecas.__main__:main - Enables both 'keecas' command and 'python -m keecas' execution - Preserves lightweight fast-path for --version and --help Tested: - keecas --version (fast, no heavy imports) - keecas --help (fast, no heavy imports) - keecas config path (full CLI with dependencies) - python -m keecas --version (module execution) Fixes module resolution issue from PR #70. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Restructure __init__.py to use __getattr__ for lazy loading of heavy dependencies (sympy, pint, pipe_command). This dramatically reduces package import time and makes CLI commands nearly instant. Performance Improvements: - import keecas: 0 sympy/pint imports (vs ALL before) - keecas --version: 45ms (vs ~2000ms before) = 44x faster - keecas config path: 88ms (vs ~2000ms+ before) = 23x faster Implementation Details: - ALL exports except __version__ are now lazy-loaded via __getattr__() - Heavy dependencies (sympy, pint, pipe_command) only load when accessed - Fixed config module collision (keecas.config package vs config object) - Added hasattr check to distinguish config types properly - Backward compatibility maintained - all import patterns work Technical Challenges Solved: 1. Submodules (col_wrappers, formatters, display) import sympy at module level - Solution: Made ALL keecas exports lazy to prevent eager loading 2. Import "from .config.manager" adds config module to namespace - Solution: Check hasattr(globals()["config"], "display") for correct type 3. Variable scope issues in conditional imports - Solution: Use globals()["config"] instead of local config variable Testing: - All 221 tests pass - Zero sympy/pint imports on basic "import keecas" - CLI performance improved by 44x - Notebooks and star imports work correctly Documentation: - Updated CLAUDE.md with lazy import guidelines - Updated task proposal with implementation results Files Modified: - src/keecas/__init__.py: Complete restructure (217 lines) - CLAUDE.md: Added performance/lazy import section - _todo/proposal/cli-performance-lazy-imports.md: Added results Fixes CLI performance issue where even --version took 2+ seconds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Feat/lazy imports performance
version bump
Task successfully implemented with 44x performance improvement. - import keecas: 45ms (vs 2000ms) - All 221 tests passing - Ready for test release Moved to: _todo/completed/2025-12-18/
…tion - Remove duplicate get_version() function - Remove custom show_quick_help() function - Defer all functionality to cli.main() - Leverage lazy imports already implemented in cli.py - Reduce file from 66 lines to 12 lines Co-authored-by: kompre <kompre@users.noreply.github.com>
Co-authored-by: kompre <kompre@users.noreply.github.com>
refactor: simplify __main__.py to avoid code duplication
- Add pull_request_target trigger for PR opens/syncs with @claude mentions - Upgrade permissions to contents:write and pull-requests:write - Add PR details extraction step for comment-based events - Fix checkout to use PR head SHA instead of base branch - Support fork PRs with repository parameter - Increase fetch-depth to 0 for full history access This ensures Claude Code Action runs on the correct PR branch context, enabling it to make changes and push commits to PRs.
- Change trigger from pull_request to pull_request_target - Ensures workflow runs in base repo context after merge - Fixes issue where release was building from pre-merge state - Maintains proper access to secrets for PyPI publishing With pull_request trigger, the workflow runs in PR context before merge completes. Using pull_request_target ensures we build and release the actual merged code on the base branch.
The pull_request_target fix was needed for release.yml, not claude.yml. Claude workflow permissions are intentionally limited for security.
fix: add pull_request_target support and proper PR branch checkout
kompre
added
release
Code Review: Release v1.1.0SummaryThis PR introduces significant performance improvements through lazy imports and fixes critical Jupyter integration issues. The changes are well-implemented with strong adherence to the project's conventions documented in CLAUDE.md. Positive Aspects1. Excellent Performance Optimization
2. Critical Bug Fix
3. Good Documentation
4. Proper Workflow
Issues and Concerns1. Missing Test Coverage for Lazy Loading (High Priority)The lazy loading mechanism is a critical change but lacks dedicated tests. Missing test scenarios include verifying no sympy/pint imports on basic import keecas, testing that version is accessible without heavy imports, verifying lazy attributes are cached after first access, and testing all import patterns. Recommend creating tests/test_lazy_imports.py with comprehensive tests for the lazy loading mechanism. 2. Config Collision Handling Complexity (Medium Priority)The config collision check in init.py is repeated 5 times (DRY violation). Consider extracting to a helper function _ensure_config() to reduce duplication and improve maintainability. 3. CLI Entry Point Change (Low Priority - Clarification Needed)The change from keecas.cli:main to keecas.main:main adds an extra indirection layer. Can you clarify what specific issue this fixes? Both approaches should work correctly. 4. Release Workflow Security (Medium Priority)The pull_request_target trigger is safe here because it only runs on merge to protected main branch. However, recommend adding a comment in the workflow explaining the security model to prevent future misuse. Minor Issues
Overall AssessmentRating: APPROVE with required changes (after addressing issues 1-2) This is a high-quality PR that solves real performance and usability problems. The lazy loading implementation is sound, and the Jupyter fix addresses a critical bug. The main gaps are test coverage for the lazy loading mechanism and code quality for config collision handling. Once these are addressed, this is ready to merge. Excellent work on the performance optimization! |
- Add comprehensive lazy loading tests (17 tests) - Test import performance and heavy dependency isolation - Test caching, dependency resolution, and import patterns - All tests pass, verify no sympy/pint on basic import - Refactor config collision handling to reduce DRY violation - Extract repeated pattern into _ensure_config_loaded() helper - Replace 5 occurrences with single function call - Reduces code from 223 to 209 lines, improves maintainability - Add comprehensive security documentation to release workflow - Explain why pull_request_target is safe for releases - Document security model for future maintainers - Clarify that only merged code is published All 238 tests pass. CLI startup time verified at ~51ms (< 100ms threshold). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
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.
Summary
This release brings significant performance improvements to the CLI and fixes critical issues with Jupyter integration and CI/CD workflows.
Major Features
Performance Improvements
keecas --versionimport keecas,from keecas import u,from keecas import *CLI Improvements
__main__.pyto defer to CLI and avoid code duplicationBug Fixes
CI/CD Improvements
Breaking Changes
None - this release maintains full backward compatibility.
Testing
time keecas --version🤖 Generated with Claude Code