-
Notifications
You must be signed in to change notification settings - Fork 0
add new composite actions, add a stale stubs check to type check workflow, refactor shared logic into composite actions #162
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
Changes from 3 commits
8752a89
ca2691f
b71a375
7f5cbab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| name: Check file diffs | ||
| description: Compare files with git diff and produce a patch if they differ | ||
| inputs: | ||
| compare_paths: | ||
| description: Space-separated list of files/paths to compare with git diff | ||
| required: true | ||
| patch_path: | ||
| description: Path to write the patch file | ||
| required: false | ||
| default: file_diff.patch | ||
| error_message: | ||
| description: Custom error message to display if files differ | ||
| required: false | ||
| default: "Files have changed" | ||
| runs: | ||
| using: composite | ||
| steps: | ||
| - id: check | ||
| name: Check for diffs and write patch | ||
| shell: bash | ||
| run: | | ||
| set -e | ||
| rc=0 | ||
| git --no-pager diff --exit-code ${{ inputs.compare_paths }} > ${{ inputs.patch_path }} 2> file_diff.err || rc=$? | ||
| if [ "$rc" -eq 0 ]; then | ||
| echo "changed=false" >> $GITHUB_OUTPUT | ||
| rm -f file_diff.err || true | ||
| elif [ "$rc" -eq 1 ]; then | ||
| echo "changed=true" >> $GITHUB_OUTPUT | ||
| echo "Patch size: $(wc -c < ${{ inputs.patch_path }}) bytes" | ||
| echo "${{ inputs.error_message }}" | ||
| echo "--- Diff ---" | ||
| cat ${{ inputs.patch_path }} || true | ||
| rm -f file_diff.err || true | ||
| else | ||
| echo "git diff failed with exit code $rc" >&2 | ||
| echo "--- git diff stderr ---" >&2 | ||
| cat file_diff.err >&2 || true | ||
| exit $rc | ||
| fi | ||
| echo "patch_path=${{ inputs.patch_path }}" >> $GITHUB_OUTPUT | ||
|
|
||
| outputs: | ||
| changed: | ||
| description: 'true if files differ' | ||
| value: ${{ steps.check.outputs.changed }} | ||
| patch_path: | ||
| description: 'path to generated patch file' | ||
| value: ${{ steps.check.outputs.patch_path }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| name: "Install CI Dependencies" | ||
| description: "Install Python dependencies for CI workflows including platform-dependent packages and post-upgrades" | ||
|
|
||
| inputs: | ||
| show_pip_list: | ||
| description: "Whether to show pip list output after installations" | ||
| required: false | ||
| default: "false" | ||
| apply_post_upgrades: | ||
| description: "Whether to apply post-upgrade packages" | ||
| required: false | ||
| default: "true" | ||
|
|
||
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - name: Set up venv and install ci dependencies | ||
| shell: bash | ||
| run: | | ||
| python -m pip install --upgrade pip setuptools wheel build --cache-dir "$PIP_CACHE_DIR" | ||
| # Prefer CI pinned requirements if present | ||
| if [ -f requirements/ci/requirements.txt ]; then | ||
| pip install -r requirements/ci/requirements.txt --cache-dir "$PIP_CACHE_DIR" | ||
| python -m pip install -e '.[test,examples,lightning]' --cache-dir "$PIP_CACHE_DIR" | ||
| else | ||
| python -m pip install -e '.[test,examples,lightning]' -c requirements/ci_constraints.txt --cache-dir "$PIP_CACHE_DIR" | ||
| fi | ||
| if [ "${{ inputs.show_pip_list }}" = "true" ]; then | ||
| pip list | ||
| fi | ||
|
|
||
| - name: Install platform-dependent packages | ||
| shell: bash | ||
| run: | | ||
| # Install platform-dependent packages with flexible constraints to allow platform-specific resolution | ||
| if [ -f requirements/platform_dependent.txt ] && [ -s requirements/platform_dependent.txt ]; then | ||
| echo "Installing platform-dependent packages..." | ||
| python -m pip install -r requirements/platform_dependent.txt --cache-dir "$PIP_CACHE_DIR" || echo "Some platform-dependent packages may not be available on this platform, continuing..." | ||
| else | ||
| echo "No platform-dependent packages to install." | ||
| fi | ||
|
|
||
| - name: Optional post-upgrades (datasets/fsspec etc) | ||
| shell: bash | ||
| env: | ||
| APPLY_POST_UPGRADES: ${{ inputs.apply_post_upgrades }} | ||
| run: | | ||
| if ([ "${APPLY_POST_UPGRADES}" = "1" ] || [ "${APPLY_POST_UPGRADES}" = "true" ]) && [ -s requirements/post_upgrades.txt ]; then | ||
| echo "Applying post-upgrades..." | ||
| python -m pip install --upgrade -r requirements/post_upgrades.txt --cache-dir "$PIP_CACHE_DIR" | ||
| if [ "${{ inputs.show_pip_list }}" = "true" ]; then | ||
| pip list | ||
| fi | ||
| else | ||
| echo "Skipping post-upgrades (either disabled or file empty)." | ||
| fi | ||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||
| name: Type Check | ||||
| name: Stale Stubs and Type Checks | ||||
|
|
||||
| on: | ||||
| push: | ||||
|
|
@@ -9,6 +9,7 @@ on: | |||
| - "pyproject.toml" | ||||
| - "src/**" | ||||
| - "requirements/**" | ||||
| - "scripts/generate_op_stubs.py" | ||||
| - ".github/workflows/type-check.yml" | ||||
| # Exclude documentation-only files from triggering | ||||
| - "!docs/**" | ||||
|
|
@@ -28,6 +29,7 @@ on: | |||
| - "pyproject.toml" | ||||
| - "src/**" | ||||
| - "requirements/**" | ||||
| - "scripts/generate_op_stubs.py" | ||||
| - ".github/workflows/type-check.yml" | ||||
| # Exclude documentation-only files from triggering | ||||
| - "!docs/**" | ||||
|
|
@@ -75,42 +77,48 @@ jobs: | |||
| restore-keys: | | ||||
| ubuntu-22.04-pip-wheels-${{ steps.set_time_period.outputs.TIME_PERIOD }}-py3.12- | ||||
|
|
||||
| - name: Set up venv and install ci dependencies | ||||
| shell: bash | ||||
| run: | | ||||
| python -m pip install --upgrade pip setuptools wheel build --cache-dir "$PIP_CACHE_DIR" | ||||
| # Prefer CI pinned requirements if present | ||||
| if [ -f requirements/ci/requirements.txt ]; then | ||||
| pip install -r requirements/ci/requirements.txt --cache-dir "$PIP_CACHE_DIR" | ||||
| python -m pip install -e '.[test,examples,lightning]' --cache-dir "$PIP_CACHE_DIR" | ||||
| else | ||||
| python -m pip install -e '.[test,examples,lightning]' -c requirements/ci_constraints.txt --cache-dir "$PIP_CACHE_DIR" | ||||
| fi | ||||
| - name: Install CI dependencies | ||||
| uses: ./.github/actions/install-ci-dependencies | ||||
| with: | ||||
| apply_post_upgrades: ${{ vars.APPLY_POST_UPGRADES || 'true' }} | ||||
|
|
||||
| - name: Install platform-dependent packages | ||||
| - name: Check type stubs are up to date | ||||
| id: check_stubs | ||||
| continue-on-error: true | ||||
| shell: bash | ||||
| run: | | ||||
| # Install platform-dependent packages with flexible constraints to allow platform-specific resolution | ||||
| if [ -f requirements/platform_dependent.txt ] && [ -s requirements/platform_dependent.txt ]; then | ||||
| echo "Installing platform-dependent packages..." | ||||
| python -m pip install -r requirements/platform_dependent.txt --cache-dir "$PIP_CACHE_DIR" || echo "Some platform-dependent packages may not be available on this platform, continuing..." | ||||
| else | ||||
| echo "No platform-dependent packages to install." | ||||
| fi | ||||
| # Make a backup of current stubs | ||||
| cp src/interpretune/__init__.pyi src/interpretune/__init__.pyi.backup | ||||
|
||||
| cp src/interpretune/__init__.pyi src/interpretune/__init__.pyi.backup |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -255,6 +255,11 @@ def generate_stubs(yaml_path: Path, output_path: Path) -> None: | |||||||||||||||||||||||||||||||||||||||||
| "from interpretune.base.datamodules import ITDataModule as ITDataModule", | ||||||||||||||||||||||||||||||||||||||||||
| "from interpretune.base.components.mixins import MemProfilerHooks as MemProfilerHooks", | ||||||||||||||||||||||||||||||||||||||||||
| "from interpretune.analysis.ops import AnalysisBatch as AnalysisBatch", | ||||||||||||||||||||||||||||||||||||||||||
| "from interpretune.analysis import (", | ||||||||||||||||||||||||||||||||||||||||||
| " AnalysisStore as AnalysisStore,", | ||||||||||||||||||||||||||||||||||||||||||
| " DISPATCHER as DISPATCHER,", | ||||||||||||||||||||||||||||||||||||||||||
| " SAEAnalysisTargets as SAEAnalysisTargets,", | ||||||||||||||||||||||||||||||||||||||||||
| ")", | ||||||||||||||||||||||||||||||||||||||||||
| "from interpretune.config import (", | ||||||||||||||||||||||||||||||||||||||||||
| " ITLensConfig as ITLensConfig,", | ||||||||||||||||||||||||||||||||||||||||||
| " SAELensConfig as SAELensConfig,", | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -264,7 +269,11 @@ def generate_stubs(yaml_path: Path, output_path: Path) -> None: | |||||||||||||||||||||||||||||||||||||||||
| " GenerativeClassificationConfig as GenerativeClassificationConfig,", | ||||||||||||||||||||||||||||||||||||||||||
| " BaseGenerationConfig as BaseGenerationConfig,", | ||||||||||||||||||||||||||||||||||||||||||
| " HFGenerationConfig as HFGenerationConfig,", | ||||||||||||||||||||||||||||||||||||||||||
| " SAELensFromPretrainedConfig as SAELensFromPretrainedConfig,", | ||||||||||||||||||||||||||||||||||||||||||
| " AnalysisCfg as AnalysisCfg,", | ||||||||||||||||||||||||||||||||||||||||||
| ")", | ||||||||||||||||||||||||||||||||||||||||||
| "from interpretune.session import ITSessionConfig as ITSessionConfig, ITSession as ITSession", | ||||||||||||||||||||||||||||||||||||||||||
| "from interpretune.runners import AnalysisRunner as AnalysisRunner", | ||||||||||||||||||||||||||||||||||||||||||
| "from interpretune.utils import rank_zero_warn as rank_zero_warn, sanitize_input_name as sanitize_input_name", | ||||||||||||||||||||||||||||||||||||||||||
| "from interpretune.protocol import STEP_OUTPUT as STEP_OUTPUT", | ||||||||||||||||||||||||||||||||||||||||||
| "", | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -296,6 +305,39 @@ def generate_stubs(yaml_path: Path, output_path: Path) -> None: | |||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| print(f"Stubs generated at {output_path}") | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| # Apply formatting to match pre-commit hooks | ||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||
| import subprocess | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| # Ensure pre-commit hooks are installed | ||||||||||||||||||||||||||||||||||||||||||
| # We run install every time since it's idempotent and fast if already installed | ||||||||||||||||||||||||||||||||||||||||||
| install_result = subprocess.run(["pre-commit", "install"], capture_output=True, text=True, cwd=project_root) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if install_result.returncode != 0: | ||||||||||||||||||||||||||||||||||||||||||
| print(f"Warning: Failed to install pre-commit hooks: {install_result.stderr}") | ||||||||||||||||||||||||||||||||||||||||||
| print("Skipping formatting step") | ||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+311
to
+319
|
||||||||||||||||||||||||||||||||||||||||||
| # Ensure pre-commit hooks are installed | |
| # We run install every time since it's idempotent and fast if already installed | |
| install_result = subprocess.run(["pre-commit", "install"], capture_output=True, text=True, cwd=project_root) | |
| if install_result.returncode != 0: | |
| print(f"Warning: Failed to install pre-commit hooks: {install_result.stderr}") | |
| print("Skipping formatting step") | |
| return | |
| import os | |
| # Ensure pre-commit hooks are installed only if not already present | |
| hook_path = project_root / ".git" / "hooks" / "pre-commit" | |
| if not hook_path.exists(): | |
| install_result = subprocess.run(["pre-commit", "install"], capture_output=True, text=True, cwd=project_root) | |
| if install_result.returncode != 0: | |
| print(f"Warning: Failed to install pre-commit hooks: {install_result.stderr}") | |
| print("Skipping formatting step") | |
| return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition logic is unnecessarily complex with nested brackets. Consider simplifying to
if [[ "${APPLY_POST_UPGRADES}" =~ ^(1|true)$ ]] && [[ -s requirements/post_upgrades.txt ]]; thenfor better readability.