Skip to content

Conversation

@nllong
Copy link
Member

@nllong nllong commented Dec 9, 2025

No description provided.

Copy link

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 introduces static type checking with mypy and refactors the codebase to use constants, custom exceptions, and structured logging. The changes include adding type hints, replacing magic numbers with named constants, introducing custom exception classes for better error handling, and implementing a logging mixin for consistent logging across classes.

  • Adds comprehensive mypy configuration with gradual adoption approach
  • Introduces three new modules: constants.py, exceptions.py, and logging_config.py
  • Refactors existing code to use type hints, constants, and improved error handling

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
urbanopt_des/constants.py New module defining constants for physical values, file extensions, resampling intervals, and output file names
urbanopt_des/exceptions.py New module introducing custom exception hierarchy for better error handling and debugging
urbanopt_des/logging_config.py New module providing logging configuration and a LoggingMixin class for consistent logging
urbanopt_des/modelica_results.py Extensive refactoring with type hints, logging integration, better error handling, and use of constants; includes bug fixes for pandas FutureWarnings
urbanopt_des/urbanopt_results.py Adds type hints and improves pandas indexing patterns; adds copyright header and mypy disable comments
urbanopt_des/urbanopt_analysis.py Improves type safety with explicit float conversions and better pandas indexing; adds type hints for empty dictionaries
urbanopt_des/results_base.py Adds class-level type annotations and improves return type hints for better type checking
pyproject.toml Configures mypy with permissive settings for gradual adoption while excluding specific directories
.github/workflows/ci.yml Adds mypy type checking step to CI pipeline to enforce type safety
Comments suppressed due to low confidence (1)

urbanopt_des/modelica_results.py:708

  • Logical error: the condition uses or but should use and. The expression df_resampled.shape[0] != 8760 or df_resampled.shape[0] != 8760 * 4 will always evaluate to True since a number cannot be both 8760 and 35040. Change to: df_resampled.shape[0] != 8760 and df_resampled.shape[0] != 8760 * 4.
        df_resampled[numeric_columns] = df_resampled[numeric_columns].interpolate(method="linear", inplace=False)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

nllong and others added 6 commits December 9, 2025 17:26
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@nllong nllong requested a review from kflemin December 10, 2025 00: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.

2 participants