Skip to content

ENH 4.2|4.3 CI/CD Enhancements: pyproject.toml + linting + pre-commit hooks#16

Open
xandie985 wants to merge 8 commits intomainfrom
issue-13/cicd-enhancements-2
Open

ENH 4.2|4.3 CI/CD Enhancements: pyproject.toml + linting + pre-commit hooks#16
xandie985 wants to merge 8 commits intomainfrom
issue-13/cicd-enhancements-2

Conversation

@xandie985
Copy link
Copy Markdown
Collaborator

@xandie985 xandie985 commented Jul 10, 2025

Changes


  • Dependency Cleanup
  • Moved dev/test-only dependencies to optional groups in pyproject.toml.
  • CI/CD Fixes
  • Enabled linting (ruff) in GitHub Actions.
  • Pre-commit Setup
  • Added hooks for linting, formatting, and code hygiene.

@xandie985 xandie985 self-assigned this Jul 10, 2025
@xandie985 xandie985 added documentation Improvements or additions to documentation ci/cd for tasks related to ci/cd labels Jul 10, 2025
@xandie985 xandie985 requested a review from Copilot July 11, 2025 09:34
Copy link
Copy Markdown

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 enhances the project’s CI/CD pipeline and development setup by organizing dependencies, restoring linting, and adding pre-commit hooks.

  • Reorganized pyproject.toml: moved dev/test dependencies to optional groups, pinned project metadata, and added coverage, mypy, and Ruff configurations.
  • Enabled Ruff linting in GitHub Actions, running only on changed Python files.
  • Added a .pre-commit-config.yaml and updated the README with installation steps, dependency groups, and pre-commit hook setup.

Reviewed Changes

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

File Description
pyproject.toml Split dependencies into core and optional groups; added tooling config (Ruff, coverage, mypy)
README.md Expanded installation instructions, dependency extras, and pre-commit usage docs
.pre-commit-config.yaml New pre-commit hooks for Ruff, formatting, whitespace, and other checks
.github/workflows/python-package.yml Restored and updated the lint job to run Ruff only on changed files
Comments suppressed due to low confidence (2)

.github/workflows/python-package.yml:45

  • The test job is indented more deeply than the lint job under jobs:. It should be aligned at the same level as lint (two spaces under jobs:) to avoid YAML parsing errors.
  test:

pyproject.toml:143

  • The coverage exclude pattern won’t match the typical Python guard if __name__ == "__main__":. Update the string to include escaped quotes or a proper regex, e.g., "if __name__ == \"__main__\"":.
    "if __name__ == .__main__.:",

@xandie985 xandie985 changed the title ENH 4.2|4.3 CI/CD Enhancements & Test Restoration v2 ENH 4.2|4.3 CI/CD Enhancements: pyproject.toml + linting + pre-commit hooks Jul 14, 2025
@xandie985
Copy link
Copy Markdown
Collaborator Author

Hi @fkiraly , this MR is ready to review. For ease of review, I already made copilot review the code and there are some low profile comments. So, this is ready for your review :)

Copy link
Copy Markdown

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

CI

  • Does the lint job run as expected, i.e., it fails if the files are no tlinted?
    Would it be perhaps simpler to run pre-commit, given that you have now added a config for it? E.g.,
      - name: Run pre-commit on changed files
        run: |
          if [ -n "$CHANGED_FILES" ]; then
            pre-commit run --color always --files $CHANGED_FILES --show-diff-on-failure
          else
            echo "No changed files to check."
          fi
  • should test run over a wider matrix of python versions?
  • if you move to pyproject, then the CI install should also use pyproject and not requirements.txt
  • is build needed in the PR CI? I thought this is primarily for release.

pyproject

  • looks overall reasonable
  • python 3.8 is deprecated, 3.9 is supported
  • 3.13 is also a currently released version
  • are all the core dependencies needed for base functionality?

@xandie985
Copy link
Copy Markdown
Collaborator Author

CI

  • Does the lint job run as expected, i.e., it fails if the files are no tlinted?

Yes, the lint job will fail if the recommit detects any issues in the files changed. Also --show-diff-no-failure will provide clear output output in the CI logs.

Would it be perhaps simpler to run pre-commit, given that you have now added a config for it? E.g.,

      - name: Run pre-commit on changed files
        run: |
          if [ -n "$CHANGED_FILES" ]; then
            pre-commit run --color always --files $CHANGED_FILES --show-diff-on-failure
          else
            echo "No changed files to check."
          fi

This has been integrated now, the lint job would will directly run on pre-commit on changed files.

  • should test run over a wider matrix of python versions?

We are running this only on pyhon 3.12 version (could be expanded over multiple versions if needed lateron)

  • if you move to pyproject, then the CI install should also use pyproject and not requirements.txt

Yes, now all the CI jobs (lint, test, docs and new build/publish workflows) are using pyproject.toml and uv.

  • is build needed in the PR CI? I thought this is primarily for release.

Yes, from the broader pov, this should have been added to a separate workflow apart from the main workflow. Now,

pyproject

  • looks overall reasonable
  • python 3.8 is deprecated, 3.9 is supported
  • 3.13 is also a currently released version

Currently I have set up the CIs for 3.12 version, will it be better to extend to 3.13 as well (or shall we keep this for later update)

  • are all the core dependencies needed for base functionality?

Seggregating the minimal set was a little challenging, and based on the errors encountered while running the core workflow, I have added these libraries. If there are any issues in particular, we can discuss about them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd for tasks related to ci/cd documentation Improvements or additions to documentation

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants