Skip to content

refactor(cli): reorganize parameters module and eliminate code duplication#115

Merged
ryanmccann1024 merged 27 commits intorelease/6.0.0from
refactor/cli-organization
Sep 10, 2025
Merged

refactor(cli): reorganize parameters module and eliminate code duplication#115
ryanmccann1024 merged 27 commits intorelease/6.0.0from
refactor/cli-organization

Conversation

@ryanmccann1024
Copy link
Copy Markdown
Collaborator

Description:
This PR completes the CLI organization refactor by restructuring the parameters module for better maintainability and eliminating code duplication across CLI entry points. The work builds on previous CLI modernization efforts to achieve perfect pylint compliance and improved code organization.

🔧 Type of Change

Primary Change Type:

  • 🐛 Bug Fix - Non-breaking change that fixes an issue
  • New Feature - Non-breaking change that adds functionality
  • 💥 Breaking Change - Change that would cause existing functionality to break
  • 🔄 Refactor - Code change that neither fixes a bug nor adds a feature
  • 📚 Documentation - Documentation only changes
  • 🧪 Tests - Adding missing tests or correcting existing tests
  • 🏗️ Build/CI - Changes to build process or CI configuration
  • 🎨 Style - Code style changes (formatting, missing semicolons, etc.)
  • Performance - Performance improvements
  • 🔒 Security - Security vulnerability fixes

Component(s) Affected:

  • CLI Interface (fusion/cli/)
  • Configuration System (fusion/configs/)
  • Simulation Core (fusion/core/)
  • ML/RL Modules (fusion/modules/rl/, fusion/modules/ml/)
  • Routing Algorithms (fusion/modules/routing/)
  • Spectrum Assignment (fusion/modules/spectrum/)
  • SNR Calculations (fusion/modules/snr/)
  • Visualization (fusion/visualization/)
  • GUI Interface (fusion/gui/)
  • Unity/HPC Integration (fusion/unity/)
  • Testing Framework (tests/)
  • Documentation
  • GitHub Workflows (.github/)
  • Build/Dependencies

🧪 Testing

Test Coverage:

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • Existing tests still pass
  • Performance impact assessed

Test Details:

  • All existing CLI tests pass (5/5 test cases)
  • Manual verification of CLI entry points and utilities
  • Pylint validation achieving perfect 10.00/10 score
  • Import verification for all refactored modules

Commands to Reproduce Testing:

# Run CLI argument tests
python -m pytest tests/test_parse_args.py tests/test_config_args.py -v

# Verify pylint compliance
python -m pylint fusion/cli/ --output-format=text

# Test CLI utilities functionality
python -c "from fusion.cli.utils import create_entry_point_wrapper, create_main_wrapper; print('✅ All utilities work')"

# Verify parameter imports
python -c "from fusion.cli.parameters import *; print('All imports successful')"

Test Results:

  • Operating System: macOS (Darwin 24.5.0)
  • Python Version: 3.11.9
  • Test Environment: Local development environment

📊 Impact Analysis

Performance Impact:

  • No performance impact
  • Performance improved
  • Minor performance decrease (acceptable)
  • Significant performance impact (needs discussion)

Memory Usage:

  • No change in memory usage
  • Memory usage optimized
  • Minor increase in memory usage
  • Significant memory impact

Backward Compatibility:

  • Fully backward compatible
  • Minor breaking changes with migration path
  • Major breaking changes (requires version bump)

Dependencies:

  • No new dependencies
  • New dependencies added (list in Additional Notes)
  • Dependencies removed/updated

🔄 Migration Guide

Breaking Changes (if any):
None - all changes maintain backward compatibility

Migration Steps:
No migration required - all existing code continues to work unchanged

✅ Code Quality Checklist

Architecture & Design:

  • Follows established architecture patterns
  • Code is modular and follows separation of concerns
  • Interfaces are well-defined and documented
  • Error handling is comprehensive
  • Logging is appropriate and informative

Code Standards:

  • Code follows project style guidelines
  • Variable and function names are descriptive
  • Code is properly commented
  • Complex logic is documented
  • No dead code or unused imports

Configuration & CLI:

  • CLI arguments follow established patterns
  • Configuration validation updated (if needed)
  • Schema updated for new config options
  • Backward compatibility maintained for configs

Security:

  • No sensitive information hardcoded
  • Input validation performed where needed
  • No security vulnerabilities introduced
  • Dependencies scanned for vulnerabilities

📚 Documentation

Documentation Updates:

  • Code comments added/updated
  • API documentation updated
  • User guide/tutorial updated
  • Configuration reference updated
  • CHANGELOG.md updated
  • README updated (if needed)

Examples Added:

  • Usage examples in docstrings
  • Configuration examples
  • CLI usage examples
  • Integration examples

🚀 Deployment

Deployment Considerations:

  • Safe to deploy to all environments
  • Requires environment-specific configuration
  • Needs database migration (if applicable)
  • Requires manual steps (document below)

🔍 Review Guidelines

For Reviewers:

  • PR description is clear and complete
  • Code changes align with described functionality
  • Tests are comprehensive and pass
  • Documentation is adequate
  • No obvious security issues
  • Performance impact is acceptable

Review Focus Areas:

  • CLI parameter module organization and naming consistency
  • Shared utilities implementation and usage patterns
  • Sphinx documentation compliance across all modules
  • Import paths and module structure changes

📝 Additional Notes

Key Commits in this Branch:

  1. 5d617c6 - refactor: create shared CLI utilities to eliminate code duplication

    • Added fusion/cli/utils.py with reusable entry point patterns
    • Refactored CLI modules to use shared utilities
    • Eliminated ~75 lines of duplicate boilerplate code
    • Achieved perfect 10.00/10 pylint score
  2. 4ab2367 - refactor: rename CLI args directory to parameters with improved organization

    • Renamed fusion/cli/args/fusion/cli/parameters/
    • Split monolithic simulation_args.py into focused modules
    • Removed redundant _args suffix from all files
    • Improved Sphinx documentation standards
    • Added missing egn_model parameter
  3. Previous commits (from earlier work on this branch):

    • 0ac76a9 - docs: clean up docstrings and remove repetitive comments
    • 7e1568b - refactor: update CLI package interface with modern functions
    • 5ac7825 - refactor: modernize CLI entry points with enhanced error handling
    • 6f7091b - refactor: modernize main_parser.py with improved naming and docs
    • And earlier commits establishing the CLI architecture

Quality Metrics:

  • Pylint Score: 10.00/10 (perfect compliance)
  • Test Coverage: 5/5 CLI tests passing
  • Code Reduction: ~75 lines of duplicate code eliminated
  • Documentation: Full Sphinx compliance across all modules

Open Questions:
None - implementation is complete and tested

Future Work:

  • Consider adding more comprehensive CLI integration tests
  • Potential for further parameter module organization as new features are added

🏁 Final Checklist

Before submitting this PR, confirm:

  • I have followed the contributing guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

ryanmccann1024 and others added 14 commits August 13, 2025 11:14
Architecture Plan Phase 2 - Core Decoupling & Simulation Pipeline Implementation
refactor(interfaces): implement abstract interfaces and complete modular architecture migration
- Fixed TypeError in cross-platform compatibility test
- Changed create_input call to use proper parameter names (base_fp, engine_props)
- Added default 'data' value for base_fp parameter

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Added thread_num, date, and sim_start fields to sim_params
- Fixes KeyError in cross-platform CI tests
- Uses default values when fields are not present in config

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Create multiprocessing.Event stop_flag when not provided
- Fixes AttributeError in cross-platform CI tests
- Ensures SimulationEngine has required stop_flag for proper execution

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Added 'seeds' parameter to engine_props (defaults to None)
- Removed Unicode emojis from error messages to fix Windows encoding
- Fixes KeyError and UnicodeEncodeError in cross-platform CI tests

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

Co-Authored-By: Claude <noreply@anthropic.com>
Extract configuration-related code into focused modules:
- fusion/configs/errors.py: Custom configuration exceptions
- fusion/configs/constants.py: Configuration constants and defaults
- fusion/configs/schema.py: Configuration option schemas with types
- fusion/utils/config.py: Reusable configuration utility functions

Improves code organization, reduces duplication, and enhances
maintainability by separating concerns and creating testable units.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Major improvements to fusion/cli/config_setup.py:
- Add comprehensive Sphinx-style docstrings for all public functions
- Implement specific exception handling with custom ConfigError types
- Extract helper functions to reduce code duplication
- Add proper type annotations throughout
- Improve variable naming for clarity (other_dict → optional_dict)
- Replace broad exception catching with specific error types
- Reduce file size from 660+ to ~400 lines through better organization

Maintains backward compatibility while significantly improving
code quality, readability, and maintainability.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Update import statements across codebase to reference the new
configuration module structure:
- tests/test_config_args.py: Import from fusion.configs.schema
- fusion/unity/make_manifest.py: Update schema imports

Ensures all code continues to work with the refactored configuration
modules while maintaining existing functionality.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Create CODING_STANDARDS.md with detailed guidelines covering:
- Naming conventions (functions, variables, classes, constants)
- Data type suffix recommendations for clarity (_dict, _set, etc.)
- Code organization and module structure best practices
- Documentation standards using Sphinx format
- Error handling patterns and custom exceptions
- Type annotation requirements and best practices
- Import organization and testing standards
- Performance and security guidelines

Provides a comprehensive reference for consistent code quality
across the FUSION project codebase.

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

Co-Authored-By: Claude <noreply@anthropic.com>
@ryanmccann1024 ryanmccann1024 self-assigned this Sep 9, 2025
ryanmccann1024 and others added 8 commits September 9, 2025 14:10
Major improvements to fusion/cli/main_parser.py:
- Add descriptive function names following coding standards
  (build_main_argument_parser, create_training_argument_parser, etc.)
- Extract constants for argument group configurations
- Add comprehensive Sphinx-style docstrings
- Add complete type annotations throughout
- Maintain backward compatibility with legacy function names
- Improve variable naming with data type clarity

All legacy functions maintained through compatibility aliases
ensuring zero breaking changes to existing code.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Major improvements to run_gui.py, run_sim.py, and run_train.py:
- Replace broad exception catching with specific exception types
- Add comprehensive error handling with actionable user guidance
- Add proper type annotations and Sphinx-style docstrings
- Create shared CLI constants module for exit codes
- Improve variable naming for clarity (args → gui_arguments, etc.)
- Remove repetitive comments and architecture boilerplate
- Add helpful error messages with installation commands
- Maintain backward compatibility functions

Features enhanced user experience with:
- Visual feedback using emojis (🛑, ❌, 💡)
- Specific error messages for common issues
- Actionable troubleshooting suggestions
- Clean separation of main logic and process exit handling

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

Co-Authored-By: Claude <noreply@anthropic.com>
Update fusion/cli/__init__.py to expose new function names and constants:
- Add modern function imports (build_main_argument_parser, etc.)
- Export shared CLI constants (SUCCESS_EXIT_CODE, ERROR_EXIT_CODE)
- Maintain legacy function exports for backward compatibility
- Update package documentation to reflect new constants module
- Organize imports with clear modern/legacy separation

Provides clean migration path while maintaining zero breaking changes
for existing code using the CLI package interface.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Polish documentation across CLI module:
- Remove repetitive "architecture best practice" phrases from docstrings
- Clean up redundant delegate comments that were self-evident
- Improve module docstrings with specific functionality descriptions
- Maintain consistent documentation quality and tone
- Focus docstrings on what modules do rather than architectural principles

Results in cleaner, more focused documentation that provides
actual value to developers using the CLI modules.

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

Co-Authored-By: Claude <noreply@anthropic.com>
…ization

- Rename fusion/cli/args/ → fusion/cli/parameters/ for clearer naming
- Split monolithic simulation_args.py into focused modules:
  * network.py: Network topology, links, nodes, spectrum bands
  * routing.py: Routing algorithms, spectrum assignment, SNR, SDN
  * traffic.py: Traffic generation, Erlang loads, simulation control
- Rename files to remove redundant _args suffix:
  * common_args.py → shared.py
  * analysis_args.py → analysis.py
  * snr_args.py → snr.py
  * training_args.py → training.py
- Update all imports and references throughout codebase
- Improve docstrings to follow Sphinx documentation standards
- Add missing egn_model parameter to SNR configuration
- Maintain backward compatibility through registry system

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Add fusion/cli/utils.py with standardized entry point patterns
- Implement create_entry_point_wrapper() for consistent legacy functions
- Implement create_main_wrapper() for simple sys.exit wrappers
- Refactor run_gui.py, run_train.py, run_sim.py to use shared utilities
- Remove ~75 lines of duplicated boilerplate code across CLI modules
- Improve pylint score from 9.95/10 to 10.00/10
- Eliminate duplicate code warnings (R0801) completely
- Move imports to top-level and remove unused sys imports
- Maintain full backward compatibility for all entry points

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

Co-Authored-By: Claude <noreply@anthropic.com>
@ryanmccann1024 ryanmccann1024 force-pushed the refactor/cli-organization branch 2 times, most recently from af5132a to 72df3fa Compare September 9, 2025 19:13
ryanmccann1024 and others added 4 commits September 9, 2025 14:25
- Fixed TypeError in cross-platform compatibility test
- Changed create_input call to use proper parameter names (base_fp, engine_props)
- Added default 'data' value for base_fp parameter

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Added thread_num, date, and sim_start fields to sim_params
- Fixes KeyError in cross-platform CI tests
- Uses default values when fields are not present in config

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Create multiprocessing.Event stop_flag when not provided
- Fixes AttributeError in cross-platform CI tests
- Ensures SimulationEngine has required stop_flag for proper execution

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Added 'seeds' parameter to engine_props (defaults to None)
- Removed Unicode emojis from error messages to fix Windows encoding
- Fixes KeyError and UnicodeEncodeError in cross-platform CI tests

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

Co-Authored-By: Claude <noreply@anthropic.com>
@ryanmccann1024 ryanmccann1024 force-pushed the refactor/cli-organization branch from d41d29d to 5a619f7 Compare September 9, 2025 19:25
@ryanmccann1024 ryanmccann1024 merged commit 5d6e02e into release/6.0.0 Sep 10, 2025
9 checks passed
@ryanmccann1024 ryanmccann1024 deleted the refactor/cli-organization branch January 19, 2026 19:13
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