Skip to content

get all ruff pep8-naming (N) tests passing - #254

Merged
markcoletti merged 3 commits into
0.9.0-prereleasefrom
pep8-naming-and-formatting
Jul 31, 2026
Merged

get all ruff pep8-naming (N) tests passing#254
markcoletti merged 3 commits into
0.9.0-prereleasefrom
pep8-naming-and-formatting

Conversation

@Lance-Drane

@Lance-Drane Lance-Drane commented Jul 28, 2026

Copy link
Copy Markdown

Apologies for how annoying this is to review, there were a LOT of violations.

AI was pretty helpful with making the Python code consistent. I also went through and made sure that in the config files:

  • the NAME parameter of a Component was always UpperCamelCase
  • the MODULE or SCRIPT of a Component configuration was always lower_snake_case

I eventually just had AI try to fix some Ruff rules which couldn't be autofixed, this follows the pep8-naming module rules and can be checked by running ruff check --select N.

This also runs the formatter against the code because the single commit in this MR was already a mess.

Prior setup

I added a dev group to the [dependency-groups] in PyPI, if you are using UV or you have Pip >=25.1 you should be able to run uv sync or pip install --group dev respectively. This group includes ruff, pre-commit, and a couple of pytest modules (we should try managing all development dependencies here and then change all CI/CD code to install based off of that list, for more declarative configuration)

Also I have a pre-commit hook set up (run pre-commit install after you've installed pre-commit) which will auto-handle the formatting and linting, as well as a yaml/toml checker and an EOF fixer. Rules are configured in .pre-commit-config.yaml. This fails the commit if it makes any changes or has a non-zero exit code, but the formatting/fixes are auto-applied when possible, so you can just add the files to Git staging and then commit again.

Both the .pre-commit-config.yaml and pyproject dependency-groups were already pushed to the pre-release branch, see 560d76e

Signed-off-by: Lance-Drane <Lance-Drane@users.noreply.github.com>
@Lance-Drane
Lance-Drane requested a review from markcoletti July 28, 2026 07:45
Signed-off-by: Lance-Drane <Lance-Drane@users.noreply.github.com>
@markcoletti markcoletti self-assigned this Jul 29, 2026
Comment thread ipsframework/services.py
@@ -92,20 +86,19 @@ def launch(executable: Any,
:param working_dir: The working directory in which to run this task
:returns: The task name and the return value from running the binary.
"""
import logging

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We'll have to ensure this doesn't break things since launch() is invoked remotely on the worker in another process, so the module may have to be imported there. This can be easily checked by running any of the ensemble code.

# assert rm.check_core_cap(4, 4) == (False, 'insufficient')

with pytest.raises(BadResourceRequestException) as excinfo:
with pytest.raises(BadResourceRequestError) as excinfo:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is interesting that it switched from Exception to Error.

@markcoletti markcoletti left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just had a couple non-blocking comments. (Though ensembles should be tested to ensure those imports in launch() didn't break code.) Overall, these were long overdue changes, particularly getting the naming conventions PEP8 compliant. There were also some nice type hint improvements, too.

Signed-off-by: Lance-Drane <Lance-Drane@users.noreply.github.com>
@markcoletti
markcoletti merged commit 28b6af4 into 0.9.0-prerelease Jul 31, 2026
6 of 13 checks passed
@markcoletti
markcoletti deleted the pep8-naming-and-formatting branch July 31, 2026 14:05
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