Skip to content

Conversation

markurtz
Copy link
Collaborator

@markurtz markurtz commented Aug 21, 2025

Summary

This PR adds new utility modules for safe math function operations, object info extraction, and reorganizes to consolidate into the utils package.

Details

  • Move statistics classes from objects to utils: Relocates DistributionSummaryPercentilesRunningStatsStatusDistributionSummary, and TimeRunningStats from guidellm.objects.statistics to guidellm.utils.statistics
  • Add new functions.py module: Implements defensive programming utilities including safe_getattrsafe_dividesafe_multiplysafe_addsafe_format_timestamp, and all_defined for handling None values and edge cases
  • Add new mixins.py module: Provides InfoMixin class for standardized metadata extraction and object introspection across different class hierarchies
  • Enhance text.py module: Adds comprehensive documentation, format_value_display function for consistent metric formatting, and improved text processing utilities
  • Update import statements: Modifies all affected modules (benchmarkpresentation) to import statistics classes from their new location in utils
  • Remove deprecated objects package: Deletes the now-empty objects directory and associated test files
  • Add comprehensive test coverage: Includes new test suites for functions.pymixins.py, and statistics.py
  • Update __init__.py exports: Adds new utility functions and classes to the main utils package exports for easy access

Test Plan

  • Run the existing test suite to ensure no regressions from the statistics class relocation
  • Execute new test files:
    • test_functions.py - Tests for safe operation utilities
    • test_mixins.py - Tests for InfoMixin functionality
    • test_statistics.py - Comprehensive tests for statistical analysis utilities
    • test_text.py - Tests for enhanced text processing functions

Related Issues

  • Resolves #

  • "I certify that all code in this PR is my own, except as noted below."

Use of AI

  • Includes AI-assisted code completion
  • Includes code generated by an AI application
  • Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes ## WRITTEN BY AI ##)

@markurtz markurtz self-assigned this Aug 21, 2025
@markurtz markurtz changed the base branch from main to feature/refactor/utils-core August 21, 2025 01:44
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements a comprehensive refactor that consolidates statistical analysis utilities and introduces new defensive programming utilities for the GuideLLM scheduler system. The refactor moves statistics classes from the objects package to utils and adds new modules for safe operations, object introspection, and enhanced text processing.

  • Relocates statistical analysis classes from guidellm.objects.statistics to guidellm.utils.statistics
  • Adds new utility modules including functions.py for safe operations, mixins.py for object metadata extraction, and enhanced text.py with display formatting
  • Introduces comprehensive test coverage for all new utility modules and maintains backward compatibility through import updates

Reviewed Changes

Copilot reviewed 15 out of 17 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
tests/unit/utils/test_text.py Comprehensive test suite for text processing utilities including loading, filtering, and endless text generation
tests/unit/utils/test_statistics.py Complete test coverage for statistical analysis utilities moved from objects package
tests/unit/utils/test_singleton.py Test suite for singleton pattern implementations with thread safety validation
tests/unit/utils/test_registry.py Tests for registry-based object discovery and registration system
tests/unit/utils/test_pydantic_utils.py Test coverage for Pydantic model utilities and polymorphic serialization
tests/unit/utils/test_mixins.py Tests for InfoMixin object introspection capabilities
tests/unit/utils/test_functions.py Test suite for defensive programming utilities and safe operations
tests/unit/utils/test_auto_importer.py Tests for automatic module importing and discovery mechanisms
Multiple source files Implementation of new utility modules and import path updates for statistics classes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Collaborator

@jaredoconnell jaredoconnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once my concerns and Samuel's concerns are addressed, I think this is ready for merging.
I want to confirm that we're okay with switching to Python 3.10. Otherwise these changes would need to be revised. I don't love the idea of moving to 3.10.

@markurtz markurtz force-pushed the feature/refactor/utils-extended branch from d463e77 to 02f97ff Compare August 26, 2025 16:17
Base automatically changed from feature/refactor/utils-core to feature/refactor/main August 26, 2025 19:34
@markurtz markurtz force-pushed the feature/refactor/utils-extended branch from fc47105 to b70ee98 Compare August 26, 2025 19:40
@markurtz markurtz merged commit 3db57b0 into feature/refactor/main Aug 26, 2025
11 of 16 checks passed
@markurtz markurtz deleted the feature/refactor/utils-extended branch August 26, 2025 19:41
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.

3 participants