Comprehensive CI infrastructure modernization and enhancements #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Update all workflows to latest GitHub Actions (v4/v5) - Expand Python support from 3.7-3.8 to 3.8-3.12 - Add comprehensive CI workflow (ci.yml) orchestrating all checks - Add dedicated code coverage workflow with Codecov integration - Add PR comment workflow for automated result summaries - Implement pip caching for faster builds - Add parallel test execution with pytest-xdist New Features: - Dependabot configuration for automated dependency updates - CI issue template and PR template for better contributions - Development requirements file (requirements-dev.txt) - Comprehensive CI documentation (docs/CI_INFRASTRUCTURE.md) - CHANGELOG.md for tracking project changes Workflow Improvements: - python-pytest.yml: Python 3.8-3.12, coverage reporting, caching - python-yapf.yml: Updated actions, caching - python-tutorials.yml: Python 3.9-3.11, artifact uploads, error handling - python-example.yml: Python 3.9-3.11, added example_ais.py Documentation: - Added CI status badges to README - Added 'Testing and CI' section with local testing guide - Updated setup.py with Python 3.8-3.12 classifiers Breaking Changes: - Dropped Python 3.7 support (EOL June 2023) - Dropped Python 3.6 support (EOL December 2021)
…ction_post (numpy arrays for numba)
There was a problem hiding this comment.
Pull Request Overview
This PR comprehensively modernizes the CI infrastructure for the PySA project by updating GitHub Actions workflows, expanding Python version support from 3.6-3.7 to 3.8-3.12, introducing new development tooling, and adding extensive documentation. While the majority of the changes are well-structured, there are several critical issues that need to be addressed, particularly a bug in the pysa/ais.py code and the inclusion of a generated coverage file.
- Updated Python support to versions 3.8-3.12, dropping EOL versions 3.6 and 3.7
- Modernized all GitHub Actions workflows with latest action versions (v4/v5), pip caching, and improved testing strategies
- Added comprehensive CI documentation, templates, and automated dependency management via Dependabot
Reviewed Changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Updated Python version classifiers to reflect support for 3.8-3.12 and added python_requires constraint |
| pyproject.toml | New PEP 517/518 build configuration with project metadata and dependencies |
| requirements-dev.txt | New development dependencies file for testing, formatting, and examples |
| pysa/ais.py | Modified partition_function_post to convert arguments to numpy arrays (has critical indentation bug) |
| .github/workflows/python-yapf.yml | Modernized formatting workflow with updated actions and pip caching |
| .github/workflows/python-tutorials.yml | Enhanced tutorial execution with artifact uploads and improved error handling |
| .github/workflows/python-pytest.yml | Expanded test matrix to Python 3.8-3.12 with coverage reporting |
| .github/workflows/python-example.yml | Updated example validation workflow with latest actions |
| .github/workflows/pr-comment.yml | New automated PR comment workflow for CI status summaries |
| .github/workflows/coverage.yml | New dedicated coverage workflow with multiple report formats |
| .github/workflows/ci.yml | New comprehensive main CI workflow orchestrating all checks |
| .github/dependabot.yml | New Dependabot configuration for automated dependency updates |
| .github/PULL_REQUEST_TEMPLATE.md | New PR template with checklists for contributors |
| .github/ISSUE_TEMPLATE/ci_issue.yml | New issue template for CI-related problems |
| README.md | Added CI badges and comprehensive testing documentation section |
| CHANGELOG.md | New comprehensive changelog documenting all infrastructure improvements |
| docs/CI_INFRASTRUCTURE.md | New detailed CI infrastructure documentation with usage guides |
| .coverage | Generated coverage database file (should not be committed) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add test_ais.py with tests for partition_function_post, get_log_omega, and omegas_to_partition - Test that partition_function_post correctly processes multiple samples (exercises the indentation bug) - Test error handling when beta=0 is not present - Test consistency and determinism of calculations - All tests currently pass with the fixed code
The beta_idx calculation, numpy import, and log_omegas append were incorrectly dedented outside the for loop, causing only the last sample to be processed instead of all samples. Fixed by: - Moving these lines inside the for loop with proper indentation - Moving numpy import to top of function for better code organization This bug would have caused incorrect partition function calculations when processing multiple samples. The new tests in test_ais.py verify the fix works correctly. Addresses critical bug identified in PR #1 review
Added: - .coverage (coverage database) - coverage.xml (coverage report) - htmlcov/ (HTML coverage reports) - .pytest_cache/ (pytest cache directory) These files are generated during testing and should not be committed. Addresses issue identified in PR #1 review where .coverage file was accidentally committed.
Added token parameter to codecov/codecov-action@v4 in all workflows: - python-pytest.yml - ci.yml - coverage.yml The codecov-action@v4 requires a token for reliable uploads, especially for public repositories. Without it, uploads may fail silently due to continue-on-error: true settings. Addresses reviewer nitpick from PR #1
Moved pytest from runtime dependencies to dev-only: - Removed from pyproject.toml dependencies list - Removed from requirements.txt - Already properly listed in requirements-dev.txt pytest is a development/testing tool and should not be installed for all users of the package. It should only be installed when developing or testing the package. Addresses reviewer comment from PR #1
Changed Coverage Summary step to use 'coverage report' instead of re-running pytest, which is more efficient and avoids re-executing all tests just to display the summary. The .coverage file is already generated from the test run on line 30, so we can directly use 'coverage report' to display the summary. Addresses reviewer performance comment from PR #1
Removed the redundant 'import numpy as np' statement inside the partition_function_post function (line 37). Numpy is already imported at the module level (line 20), so this local import is unnecessary. Addresses reviewer comment from PR #1
Changed docstrings to raw strings (r'''...''') in: - uniform_prob_initialization - bernoulli_prob_initialization This fixes DeprecationWarning for invalid escape sequence '\pm' which is used to represent the LaTeX ± symbol. Using raw strings prevents Python from trying to interpret backslash sequences. Fixes pytest warnings from test runs
bernalde
added a commit
that referenced
this pull request
Nov 11, 2025
… README Workflow changes: - Changed artifact naming from 'tutorial-outputs-py3.9' to 'tutorial-outputs-py3-9' - Replaced periods with hyphens using replace() function - Applied to both python-tutorials.yml and ci.yml - Prevents potential issues with periods in GitHub Actions artifact names README fix: - Fixed typo: 'envinronment.yml' → 'environment.yml' Addresses reviewer nitpick from PR #1
Fixed typo in conda environment file name in README.md: - Changed 'envinronment.yml' to 'environment.yml' Note: Artifact names with periods (tutorial-outputs-py3.9) work fine in GitHub Actions despite the reviewer's concern. The replace() function is not available in GitHub Actions expressions, and periods in artifact names are commonly used without issues.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 18 out of 20 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Modernize CI workflows with updated GitHub Actions, expanded Python support, and improved testing and coverage processes. Introduce new templates for contributions and documentation, while dropping support for outdated Python versions. Enhance readability and ensure correct argument types in the codebase.