Skip to content

Conversation

@paulf81
Copy link
Collaborator

@paulf81 paulf81 commented Dec 9, 2025

This is an optional PR and built off of #83 to capture those changes in advance. It includes two components:

Include format check

As was done for Hercules in #181, adds ruff format --check to the ci file to check for formatting

Implement formatting

It is simply the result of running:

ruff format .

At the top level (note ruff check . --fix yields no changes since that was already mandatory linting)

@paulf81 paulf81 requested a review from misi9170 December 9, 2025 20:31
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 adds ruff format --check to the CI workflow and applies automated code formatting across the entire codebase. The changes are primarily cosmetic, involving whitespace adjustments, line breaks, and formatting consistency improvements.

Key Changes:

  • Modified CI workflow to include ruff format --check (changing from just ruff format to checking format compliance)
  • Applied ruff format . across all Python files to standardize formatting
  • Added performance optimization in hercules_interface.py (pre-computing LMP keys)

Reviewed changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
.github/workflows/continuous-integration-workflow.yaml Added --check flag to ruff format command in CI
tests/wake_steering_design_test.py CRITICAL BUG: Operator precedence issue at line 365-367
tests/hercules_v1_interfaces_test.py Formatting adjustments (spacing, line breaks)
tests/hercules_interface_test.py Formatting adjustments (spacing, line breaks)
tests/controller_library_test.py Formatting adjustments (spacing, line breaks)
tests/battery_test.py Formatting adjustments (spacing, line breaks)
hycon/interfaces/hercules_v1_interface.py Formatting adjustments plus comment fixes
hycon/interfaces/hercules_interface.py Formatting adjustments plus performance optimization
hycon/design_tools/wake_steering_visualization.py Formatting adjustments
hycon/design_tools/wake_steering_design.py BUGS: Malformed docstring (line 630), incorrect type annotation (line 254)
hycon/controllers/*.py BUG in battery_controller.py: Malformed docstrings (lines 133, 139)
hycon/__init__.py Removed trailing blank line
examples/*/ Formatting adjustments across all example scripts

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

@misi9170
Copy link
Collaborator

@paulf81 I think it makes more sense to have this PR built off develop, and then to have #83 afterwards? Otherwise, the formatting changes in #83 don't really follow in my mind. I think we could just remove the functional changes from battery_controller.py here, then merge this one, and then they'll be added back in in #83.

@paulf81
Copy link
Collaborator Author

paulf81 commented Dec 10, 2025

@paulf81 I think it makes more sense to have this PR built off develop, and then to have #83 afterwards? Otherwise, the formatting changes in #83 don't really follow in my mind. I think we could just remove the functional changes from battery_controller.py here, then merge this one, and then they'll be added back in in #83.

Ok I can get that done

@paulf81
Copy link
Collaborator Author

paulf81 commented Dec 10, 2025

Ok @misi9170 I removed the not-formatting changes to hercules_interface.py in #83 to keep these seperate

Copy link
Collaborator

@misi9170 misi9170 left a comment

Choose a reason for hiding this comment

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

Thanks @paulf81 ! Too many lines changed to really look carefully through them all, but I've skimmed it all and I'll just have to trust pytest, ruff, and version control :)

I'm going to make one non-compliant change and make sure it fails the ruff check; then revert that change and merge.

@misi9170 misi9170 added the package Packaging updates label Dec 10, 2025
@misi9170
Copy link
Collaborator

Tests failed as expected. Reverting and will merge

@misi9170 misi9170 merged commit 59fc803 into NREL:develop Dec 10, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package Packaging updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants