Skip to content

[codex] address merged PR review follow-ups#65

Open
zhoubot wants to merge 2 commits intomainfrom
codex/fix-merged-pr-review-followups
Open

[codex] address merged PR review follow-ups#65
zhoubot wants to merge 2 commits intomainfrom
codex/fix-merged-pr-review-followups

Conversation

@zhoubot
Copy link
Copy Markdown
Collaborator

@zhoubot zhoubot commented Apr 9, 2026

Summary

  • reorder the op_extension side-effect import to match Python import grouping in the baseline auto-mode add test
  • add an explanatory comment on the torch_npu side-effect import in the torch-jit add demo
  • clean up temporary *_gen_data.py helper scripts in tests/script/all_cpu_tests.py via try/finally
  • update tisa_coverage to use the current THISTOGRAM<HistByte::...> wrapper so the full CPU ST build passes again

Why

These are follow-ups from review comments on merged PRs #54, #55, and #57. While validating the cleanup change, the full CPU ST build also exposed a current-main compatibility break in tisa_coverage after the THISTOGRAM wrapper moved from bool template parameters to HistByte.

Validation

  • python3 -m py_compile demos/auto_mode/baseline/add/test/test.py demos/auto_mode/torch_jit/add/add_compile_and_run.py tests/script/all_cpu_tests.py
  • ruff check demos/auto_mode/baseline/add/test/test.py demos/auto_mode/torch_jit/add/add_compile_and_run.py tests/script/all_cpu_tests.py
  • python3 tests/script/all_cpu_tests.py -g Ninja -b build/cpu_review_followups

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request performs minor refactoring and cleanup across several test files. Key changes include reordering imports in Python tests, adding explanatory comments for NPU backend registration, and updating template arguments in C++ coverage tests to use explicit enum values. Additionally, the test script was updated with a try-finally block to ensure temporary files are deleted after execution. I have no feedback to provide.

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.

1 participant