Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ruff #451

Merged
merged 8 commits into from
Mar 10, 2025
Merged

Ruff #451

merged 8 commits into from
Mar 10, 2025

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Mar 8, 2025

Description

Ruff the starcheck Python.

Interface impacts

Testing

Unit tests

  • Linux
(ska3-latest) jeanconn-fido> git rev-parse HEAD
c0db4d8f34a6391880a8b6ffad3b81c1e093ac47
(ska3-latest) jeanconn-fido> pytest
====================================================== test session starts =======================================================
platform linux -- Python 3.12.8, pytest-8.3.4, pluggy-1.5.0
rootdir: /proj/sot/ska/jeanproj/git
configfile: pytest.ini
plugins: timeout-2.3.1, anyio-4.7.0
collected 14 items                                                                                                               

starcheck/tests/test_state_checks.py .............                                                                         [ 92%]
starcheck/tests/test_utils.py .                                                                                            [100%]

====================================================== 14 passed in 27.35s

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

4 recent loads run in regression against master at

https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck/starcheck-pr451/

Only minimal expected diffs seen.

@jeanconn jeanconn requested a review from Copilot March 9, 2025 00:15

Choose a reason for hiding this comment

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

PR Overview

This PR implements configuration and formatting updates for Ruff integration along with minor code style improvements across several project files. Key changes include the addition of new Ruff configuration files and GitHub workflows, updates to pre-commit hooks, and refactoring of various file formatting and import statements.

Reviewed Changes

File Description
ruff.toml, ruff-base.toml Introduce and configure Ruff linting settings.
.github/workflows/python-formatting.yml Add a GitHub Actions workflow to check code formatting using Ruff.
.pre-commit-config.yaml Configure Ruff hooks for pre-commit checks.
.github/workflows/python-linting.yml Add a GitHub Actions workflow for Ruff linting.
setup.py Minor formatting adjustment.
starcheck/server.py Update print formatting and add a comment discouraging global variable usage.
starcheck/pcad_att_check.py Update string quoting for consistency and improve formatting in tests.
starcheck/utils.py Rearranged imports and added strict control in zip iterations for consistency.
starcheck/state_checks.py Update docstrings and adjust import order.
starcheck/plot.py Reformat function parameters and remove an unused import.
starcheck/tests/* Update test functions to include strict matching in zip calls.

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

Comments suppressed due to low confidence (2)

starcheck/server.py:85

  • [nitpick] Consider refactoring the use of the global variable 'KEY' (e.g. via dependency injection) to improve code maintainability.
global KEY  # noqa: PLW0603 Using the global statement to update `KEY` is discouraged

starcheck/plot.py:7

  • [nitpick] Verify if the removal of the 'plot_compass' import from chandra_aca.plot is intentional; if not needed, ensure any related dead code is also removed.
from chandra_aca.plot import bad_acq_stars, plot_stars
@jeanconn jeanconn requested review from javierggt and taldcroft March 9, 2025 00:18
@jeanconn jeanconn marked this pull request as ready for review March 9, 2025 00:18
Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

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

I went through all the manual changes, looks good to me!

@jeanconn jeanconn merged commit 20c3222 into master Mar 10, 2025
2 checks passed
@jeanconn jeanconn deleted the ruff branch March 10, 2025 13:06
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