Skip to content

workaround for python2.5 typing origin change#675

Open
wlav wants to merge 1 commit into
Framework-R-D:mainfrom
wlav:numpy2.5-fix
Open

workaround for python2.5 typing origin change#675
wlav wants to merge 1 commit into
Framework-R-D:mainfrom
wlav:numpy2.5-fix

Conversation

@wlav

@wlav wlav commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

In numpy 2.5, the __origin__ of any typing.NDArray[] points to the generic type (ie. .typing.NDArray itself), rather than the base type (ie. numpy.ndarray). The latter is the convention for container types (e.g. types.List), the former for descriptive types like types.UnionType. It's probably an unintended consequence of this:

-NDArray: TypeAlias = np.ndarray[_AnyShape, dtype[_ScalarT]]
+type NDArray[ScalarT: np.generic] = np.ndarray[_AnyShape, np.dtype[ScalarT]]

Nevertheless, this PR supports both, regardless whether 2.5.0 is used in the wild and/or not fixed later.

  • Code
    • Updated normalize_type in plugins/python/python/phlex/_typing.py to recognize both np.ndarray and np.typing.NDArray as NumPy array origins.
    • Adjusted dtype extraction logic to handle both generic conventions:
      • np.ndarray continues to use the existing second-type-argument pattern when present.
      • np.typing.NDArray now uses its single type argument directly as the dtype.
    • This preserves compatibility with NumPy 2.5’s changed __origin__ behavior while remaining compatible with the prior behavior.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bbcda462-f169-4dc7-8d69-b9a719922f63

📥 Commits

Reviewing files that changed from the base of the PR and between 7409297 and b184a28.

📒 Files selected for processing (1)
  • plugins/python/python/phlex/_typing.py
📜 Recent review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Analyze python with CodeQL
  • GitHub Check: coverage
⚠️ CI failures not shown inline (4)

GitHub Actions: wlav checking Python code / python-check: wlav checking Python code

Conclusion: failure

View job details

##[group]Run REPO_NAME="${REPO##*/}"
 �[36;1mREPO_NAME="${REPO##*/}"�[0m
 �[36;1mif [ "success" = 'success' ] && [ "success" = 'success' ]; then�[0m
 �[36;1m  echo "✅ Python checks passed."�[0m
 �[36;1melse�[0m
 �[36;1m  echo "::error::Python checks failed. Comment '@${REPO_NAME}bot python-fix' on the PR to attempt auto-fix."�[0m

GitHub Actions: wlav checking Python code / scripts-test: wlav checking Python code

Conclusion: failure

View job details

##[group]Run codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f
 with:
   files: coverage-scripts.xml
   flags: scripts
   name: phlex-scripts-coverage
   fail_ci_if_error: false
   verbose: true
   root_dir: phlex-src
   disable_file_fixes: false
   disable_search: false
   disable_safe_directory: false
   disable_telem: false
   dry_run: false
   git_service: github
   gcov_executable: gcov
   handle_no_reports_found: false
   recurse_submodules: false
   run_command: upload-coverage
   skip_validation: false
   use_legacy_upload_endpoint: false
   use_oidc: false
   use_pypi: false
   version: latest
 env:
   CODECOV_***REDACTED*** /home/runner/work/_temp/uv-python-dir
   UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
 ##[endgroup]
 ##[group]Run missing_deps=""
 �[36;1mmissing_deps=""�[0m
 �[36;1m�[0m
 �[36;1m# Check for always-required commands�[0m
 �[36;1mfor cmd in bash git curl; do�[0m
 �[36;1m  if ! command -v "$cmd" >/dev/null 2>&1; then�[0m
 �[36;1m    missing_deps="$missing_deps $cmd"�[0m
 �[36;1m  fi�[0m
 �[36;1mdone�[0m
 �[36;1m�[0m
 �[36;1m# Check for gpg only if validation is not being skipped�[0m
 �[36;1mif [ "$INPUT_SKIP_VALIDATION" != "true" ]; then�[0m
 �[36;1m  if ! command -v gpg >/dev/null 2>&1; then�[0m
 �[36;1m    missing_deps="$missing_deps gpg"�[0m
 �[36;1m  fi�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1m# Report missing required dependencies�[0m
 �[36;1mif [ -n "$missing_deps" ]; then�[0m
 �[36;1m  echo "Error: The following required dependencies are missing:$missing_deps"�[0m
 �[36;1m  echo "Please install these dependencies before using this action."�[0m
 �[36;1m  exit 1�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mecho "All required system dependencies are available."�[0m
 shell: /usr/bin/sh -e {0}
 env:
   CODECOV_***REDACTED*** /home/runner/work/_temp/uv-python-dir
   UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
   INPUT_SKIP_VALIDATION: false
 ##[endgroup]
 All required system dependencies are available.
 ##[grou...

GitHub Actions: wlav checking Python code / 0_scripts-test.txt: wlav checking Python code

Conclusion: failure

View job details

rts.py::TestMainSarifMode::test_new_alert_returns_zero_and_writes_comment PASSED [ 23%]
 scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_absent_alert_returns_zero_and_writes_comment PASSED [ 24%]
 scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_no_alerts_returns_zero_no_comment PASSED [ 24%]
 scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_below_threshold_alert_not_reported PASSED [ 24%]
 scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_missing_sarif_exits_nonzero PASSED [ 24%]
 scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_github_output_written PASSED [ 24%]
 scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_github_output_false_when_no_alerts PASSED [ 25%]
 scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_directory_of_sarif_files PASSED [ 25%]
 scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_sarif_mode_pr_ref_produces_filtered_url PASSED [ 25%]
 scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_non_integer_pr_ref_no_filtered_url PASSED [ 25%]
 scripts/test/test_check_codeql_alerts.py::TestMainApiMode::test_api_mode_new_alert PASSED [ 25%]
 scripts/test/test_check_codeql_alerts.py::TestMainApiMode::test_api_mode_min_level_filtering PASSED [ 26%]
 scripts/test/test_check_codeql_alerts.py::TestMainApiMode::test_api_mode_github_api_error_exits_2 PASSED [ 26%]
 scripts/test/test_check_codeql_alerts.py::TestMainApiMode::test_api_mode_missing_github_repository_exits_2 PASSED [ 26%]
 scripts/test/test_check_codeql_alerts.py::TestMainApiMode::test_api_mode_skipped_when_sarif_has_baseline PASSED [ 26%]
 scripts/test/test_check_codeql_alerts.py::TestMainApiModeWithPrRef::test_api_mode_pr_ref_produces_filtered_url PASSED [ 27%]
 scripts/test/test_check_codeql_alerts.py::TestMainEntrypoint::test_entrypoint_no_alerts_exits_zero PASSED [ 27%]
 scripts/test/test_clang_tidy_check_summary.py::TestLoadDiagnostics::test_reads_...

GitHub Actions: wlav checking Python code / 1_python-check.txt: wlav checking Python code

Conclusion: failure

View job details

##[group]Run REPO_NAME="${REPO##*/}"
 �[36;1mREPO_NAME="${REPO##*/}"�[0m
 �[36;1mif [ "success" = 'success' ] && [ "success" = 'success' ]; then�[0m
 �[36;1m  echo "✅ Python checks passed."�[0m
 �[36;1melse�[0m
 �[36;1m  echo "::error::Python checks failed. Comment '@${REPO_NAME}bot python-fix' on the PR to attempt auto-fix."�[0m
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.py: Use ruff for Python formatting and linting (configured in pyproject.toml); follow Google-style docstring conventions; use line length of 99 characters; use double quotes for strings
Use from __future__ import annotations to enable deferred evaluation of type annotations (avoids forward-reference issues; Python >=3.12)
Type hints are recommended; mypy is configured in pyproject.toml
Avoid naming Python test scripts types.py or other names that shadow standard library modules, as this causes obscure import errors
Python test scripts should follow the test structure pattern: C++ driver provides data streams, Jsonnet config wires the graph, and Python script implements algorithms

**/*.py: Enforce 99-character line limit and double quotes in Python via ruff configured in pyproject.toml
Use Google-style docstrings in Python code
Use type hints in Python code and configure mypy for type checking
Use from __future__ import annotations in Python to enable deferred evaluation of type annotations
Use PEP 8 naming in Python: CapWords for classes, snake_case for everything else
When using the read tool for Python files, always use integer values for offset and limit parameters, never float/double values

Files:

  • plugins/python/python/phlex/_typing.py
🔇 Additional comments (1)
plugins/python/python/phlex/_typing.py (1)

159-169: LGTM!


📝 Walkthrough

Walkthrough

normalize_type now treats both np.ndarray and np.typing.NDArray as NumPy array annotations and extracts dtype arguments differently for each form.

Changes

NumPy array annotation handling

Layer / File(s) Summary
NumPy array origin and dtype extraction
plugins/python/python/phlex/_typing.py
normalize_type matches both np.ndarray and np.typing.NDArray, and uses different dtype-argument extraction for each convention.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title points to Python 2.5, but the change is actually a NumPy 2.5 typing-origin workaround. Rename it to mention NumPy 2.5 and the typing.NDArray origin change, e.g. "work around NumPy 2.5 typing.NDArray origin change".
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@wlav wlav requested a review from beojan June 26, 2026 23:44
@greenc-FNAL

Copy link
Copy Markdown
Contributor

21 fixed, 0 new since branch point (7409297)

✅ 21 CodeQL alerts resolved since the branch point

  • Warning # 196 actions/untrusted-checkout-toctou/critical at .github/workflows/clang-tidy-fix.yaml:109:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 227 actions/untrusted-checkout-toctou/high at .github/workflows/clang-format-fix.yaml:94:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 228 actions/untrusted-checkout-toctou/high at .github/workflows/python-fix.yaml:94:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 229 actions/untrusted-checkout/high at .github/workflows/clang-format-fix.yaml:94:9 — Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
    Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
  • Warning # 230 actions/untrusted-checkout/high at .github/workflows/python-fix.yaml:94:9 — Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
    Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
  • Warning # 231 actions/untrusted-checkout-toctou/high at .github/workflows/cmake-format-fix.yaml:94:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 232 actions/untrusted-checkout-toctou/high at .github/workflows/jsonnet-format-fix.yaml:95:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 233 actions/untrusted-checkout/high at .github/workflows/cmake-format-fix.yaml:94:9 — Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
    Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
  • Warning # 234 actions/untrusted-checkout/high at .github/workflows/jsonnet-format-fix.yaml:95:9 — Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
    Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
  • Warning # 235 actions/untrusted-checkout/medium at .github/workflows/clang-format-check.yaml:82:9 — Potential unsafe checkout of untrusted pull request on privileged workflow.
  • Warning # 236 actions/untrusted-checkout/medium at .github/workflows/actionlint-check.yaml:86:9 — Potential unsafe checkout of untrusted pull request on privileged workflow.
  • Warning # 237 actions/untrusted-checkout/medium at .github/workflows/clang-tidy-check.yaml:59:9 — Potential unsafe checkout of untrusted pull request on privileged workflow.
  • Warning # 238 actions/untrusted-checkout/medium at .github/workflows/cmake-format-check.yaml:79:9 — Potential unsafe checkout of untrusted pull request on privileged workflow.
  • Warning # 239 actions/untrusted-checkout/medium at .github/workflows/cmake-build.yaml:159:9 — Potential unsafe checkout of untrusted pull request on privileged workflow.
  • Warning # 240 actions/untrusted-checkout/medium at .github/workflows/header-guards-check.yaml:82:9 — Potential unsafe checkout of untrusted pull request on privileged workflow.
  • Warning # 241 actions/untrusted-checkout/medium at .github/workflows/jsonnet-format-check.yaml:79:9 — Potential unsafe checkout of untrusted pull request on privileged workflow.
  • Warning # 242 actions/untrusted-checkout/medium at .github/workflows/markdown-check.yaml:82:9 — Potential unsafe checkout of untrusted pull request on privileged workflow.
  • Warning # 243 actions/untrusted-checkout/medium at .github/workflows/python-check.yaml:84:9 — Potential unsafe checkout of untrusted pull request on privileged workflow.
  • Warning # 244 actions/untrusted-checkout/medium at .github/workflows/yaml-check.yaml:76:9 — Potential unsafe checkout of untrusted pull request on privileged workflow.
  • Warning # 245 actions/untrusted-checkout-toctou/high at .github/workflows/coverage.yaml:386:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • ✅ …and 1 more alerts (see Code Scanning for the full list).

Review the full CodeQL report for details.

@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
plugins/python/python/phlex/_typing.py 75.00% 1 Missing ⚠️
@@            Coverage Diff             @@
##             main     #675      +/-   ##
==========================================
+ Coverage   83.55%   83.60%   +0.04%     
==========================================
  Files         170      170              
  Lines        6283     6300      +17     
  Branches      706      708       +2     
==========================================
+ Hits         5250     5267      +17     
  Misses        810      810              
  Partials      223      223              
Flag Coverage Δ
scripts 78.86% <ø> (ø)
unittests 85.99% <75.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
plugins/python/python/phlex/_typing.py 93.15% <75.00%> (-1.22%) ⬇️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7409297...b184a28. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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