已合并
feat: add excel support #185
RuiWang_创建于 15 天前
feat: add excel support #185
已合并
RuiWang_创建于 15 天前
RuiWang_成员
15 天前

当前PR是否有AI参与:

[ ] 否
[x] 是

  1. AI Agent 平台: opencode
  2. AI 模型: glm-5.2
  3. Prompt上下文: TTK 4 通路 Excel 用例输入适配

PR功能描述 / 为什么需要这个合入**:

为 4 条测试通路(Kernel/GEIR/ACLNN/E2E)增加 Excel(.xlsx/.xlsm) 用例输入支持,与 CSV 等价、可互换。

  • 新增表格读取层 ttk/utilities/table_reader.pyread_table/read_csv_rows/resolved_sheet),CSV 走标准库、XLSX 走 openpyxl,单元格统一 str+strip 对齐 CSV 行为。
  • UniversalTestcaseFactory 新增 from_path(path, sheet),4 通路共用,模式自动识别与字段解析零改动。
  • CLI 新增 --sheet(默认首个工作表,CSV 忽略);_detect_framework 改用 read_table 兼容 xlsx。
  • InstanceBase 按后缀路由 xlsx(避免误入 zip);默认输出名嵌入实际 sheet 名(如 cases_T2_result.csv),多 sheet 互不覆盖;输出仍为 CSV。
  • SWITCHES 新增 sheet 字段;新增 openpyxl>=3.1 依赖。
  • 新增每通路多 sheet xlsx 示例及各 Case Writing / Test Guide / Task_Execution / Test_Case_Generation / AGENTS 文档说明。
  • 顺手清理 instance_base / testcase_manager / testcase_aclnn / classes / utilities.init 既有 ruff 告警(E722/B904/UP031/F841/F821/F401/F403/B007)。

该PR关联的issue

fixes #127

希望检视人员了解:

  • xlsx 单元格一律按文本处理(None→""str()+strip),与 CSV 一致;数值列建议在 Excel 中预设为文本格式,避免 01→11e-8→浮点等类型推断。
  • 同一 xlsx 用 --sheet 跑不同 sheet,输出名各异、互不覆盖。
  • dOut 从 ACLNN backward 排除名移除(代码 + 文档一致)。

测试

  • 4 通路 --validate 全过(kernel 4 / geir 2 / aclnn 2 / e2e 4 用例均 valid)。
  • kernel 端到端 ttk kernel -i examples/case_store/kernel/add.xlsx --sheet T2:编译 SUCC、精度 PASS、4/4 PASS。

文档更新

  • docs/Task_Execution.md:通用参数表新增 --sheet--input 说明改 csv/xlsx。
  • docs/Test_Case_Generation.md:新增「输入文件格式(CSV / Excel)」小节。
  • docs/Operator_Test_Guides/*_Test_Guide.md:各通路「常用场景示例」补 xlsx 多 sheet 命令。
  • docs/Operator_Test_Guides/*_Case_Writing.md:各通路「参考用例」表补每个 csv 的验证特性与关键列。
  • AGENTS.md / examples/case_store/README.md:xlsx 说明。

类型标签

likedislike
Pull Request已成功合入, 合并人@CANN-robot
(感谢 RuiWang_ 的贡献)
RRuiWang_成员
15 天前 创建了 pull request,commit 07ef2f0d
atomgit-bot
atomgit-bot
15 天前 评论:

变更摘要

本 PR 为 ttk 增加 Excel 表格(.xlsx/.xlsm)测试用例文件的支持。核心做法是引入统一的表格读取层 ttk.utilities.table_reader(提供 read_tableread_csv_rowsresolved_sheet 等函数,CSV 走原 csv 解析、XLSX 依赖新增的 openpyxl>=3.1),并让 UniversalTestcaseFactory 通过新增的 from_path 类方法按路径加载 CSV 或 XLSX 用例;CLI 的 run/list 命令新增 --sheet 参数用于指定工作表(默认第一张),框架检测 _detect_framework_from_csv 与用例加载流程统一改走表格读取接口;InstanceBase 按扩展名将 xlsx/xlsm 路由到表格加载,并在未指定输出时把工作表名嵌入默认结果文件路径,SWITCHES 新增 sheet 字段承载该配置。

主要改动

  • 新增表格读取工具与依赖:在 ttk/utilities 下引入 table_reader 模块并在 __init__.py 中导出,统一提供 CSV/XLSX 读取能力;pyproject.toml 新增 openpyxl>=3.1 依赖(并为 ttk/utilities/__init__.py 配置 ruff 的 F401/F403 忽略规则)。
  • UniversalTestcaseFactory.from_path 与读取重构:在 ttk/core_modules/testcase_manager/testcase_manager.py 中新增 from_path(path, sheet, skip_validate) 类方法,将 CSV 解析抽为 read_csv_rows,并把构造逻辑重构为 _init_common + _init_from_rows,支持按路径加载 Excel 并指定工作表。
  • CLI 支持 --sheet 参数ttk/cli/common.py_add_io_argsttk/cli/list_cmd.py 均新增 --sheet(默认首个工作表,CSV 忽略),--input 帮助文案更新为 csv/xlsx;ttk/cli/bridge.py_apply_io_argsargs.sheet 写入 sw.sheet_detect_framework_from_csv 增加 sheet 参数并改用 read_table 读取首个表格进行框架检测。
  • InstanceBase 加载与默认输出路径适配_load_cases.xlsx/.xlsm 直接走 _load_case_from_table(原 _load_case_from_csv 更名而来),_load_case_from_table 读取 sw.sheet_resolve_output_path 改用 os.path.splitext 并借助 _resolved_output_sheet 将工作表名(非法字符替换为下划线)嵌入默认结果文件名。
  • SWITCHES 新增 sheet 配置字段:在 ttk/utilities/classes.pySWITCHES 中加入 sheet 字段,并引入 TYPE_CHECKING 下的 TestcaseOp 类型标注以支持跨模块引用(其余大量改动为引号统一、换行、f-string 等格式化调整,无业务影响)。
likedislike
不准确?
atomgit-bot
atomgit-bot
15 天前 评论:

代码审查

pyproject.toml and requirements.txt changes are benign (openpyxl>=3.1, ruff per-file-ignores).

Note: [tool.ruff.lint.per-file-ignores] — this requires a newer ruff version than the older [tool.ruff.per-file-ignores] syntax. The current ruff config uses [tool.ruff.lint] (select/ignore), which is the modern schema. [tool.ruff.lint.per-file-ignores] is the correct modern form. Fine.

Now, let me review the requirements.txt — just openpyxl added. Fine.

Let me look at the remaining important detail: in _detect_framework_from_csv, the docstring change and behavior. One more check: the new function signature _detect_framework_from_csv(input_files, sheet=None) — name still says "csv" but handles xlsx too. Minor naming, not reportable.

Now, let me reconsider the most important logic change in the diff: _load_cases and _load_case_from_table. There's a subtle regression:

Previously, _load_case_from_csv used open(testcase_path, newline='', encoding='utf-8') and UniversalTestcaseFactory(file) which used csv.reader. Now _load_case_from_table uses UniversalTestcaseFactory.from_path(testcase_path, sheet)read_table(path, sheet).

If read_table for a CSV uses open(..., encoding='utf-8') without newline='', CSV line endings could be an issue on Windows — but Linux target. Can't verify (module missing).

Also, a key behavior difference: previously _load_case_from_csv stripped each cell with .strip() (in _read_csv). Now read_csv_rows must do the stripping — can't verify.

OK — the P0 dominates everything.

Now let me look for a second real issue beyond the missing module. Let me re-examine _detect_framework_from_csv behavior change on mixed-empty-rows once more, but with the actual read_table semantics unknown, I can't be sure.

Let me instead check the from_path/read_table interaction with __slots__ and the get() method — self.testcases populated in _init_from_rows via _parse_testcase... wait, _init_from_rows calls _testcase_hdr_check() and _parse_testcase(). _parse_testcase builds raw_testcases and calls self._parse(raw_testcases) which populates self.testcases. OK.

But hold on — in the OLD __init__, after _read_csv(file), it called _testcase_hdr_check() and _parse_testcase(). In the new __init__, _init_common + _init_from_rows does the same. Good.

Now — there's a subtle thing: in the new from_path, read_table(path, sheet) is called with sheet positional. If read_table's signature is read_table(path, sheet=None), fine. Can't verify.

Now let me look at whether UniversalTestcaseFactory.__init__ is still called anywhere with the file — yes, _load_case_from_zip (instance_base.py:701). That path now calls read_csv_rows(file) which must handle a TextIO. Can't verify.

OK. Let me now look at the rest of instance_base.py diff for anything I missed — particularly the removed import pathlib from _write_validate_result (it was a local import import pathlib inside the function that's now removed — but pathlib isn't used there anyway, dead import removed — fine).

Let me also double check the _write_validate_result — unchanged semantics.

Now, let me look at the _summary_print more carefully for the remain_count/loop_count logic — unchanged, just reformatted.

Let me now look at ttk/cli/bridge.py _apply_io_argssw.sheet = args.sheet only when truthy. If user passes --sheet "" (empty), it's falsy → sw.sheet stays None. Fine.

Let me check: are there OTHER CLI entry points that construct SWITCHES and need sheet in slots — e.g. validate command or run? Since sheet is in slots and init, fine.

Now, one more possible issue: In instance_base.py, self.switches.sheet is accessed via getattr(self.switches, "sheet", None) everywhere — safe even if SWITCHES lacked sheet. Fine.

Now let me reconsider the .gitignore — also check the old file had -*.log and -**/*.log. New has only **/*.log. *.log (root-level) is covered by **/*.log in gitignore? Gitignore semantics: a pattern containing a slash (other than trailing) is anchored to the .gitignore location. **/*.log — contains a slash, but **/ at the start matches in all directories including root, and * matches filenames. Per git docs, "A leading ** followed by a slash means match in all directories." So **/*.log matches any .log anywhere including root. So *.log is effectively covered. But to be safe — old had BOTH *.log and **/*.log; new has only **/*.log. Since **/*.log covers root too, no regression. But model_results/ is genuinely dropped.

Wait — is there any other dropped pattern? Let me diff old vs new patterns systematically:
Old patterns:

  1. .idea/ → new: .idea/ ✓
  2. *.pyc → new: *.pyc ✓
  3. .log → covered by **/.log ✓ (functionally)
  4. test.py → new ✓
  5. **/*.log → new ✓
  6. tests/**/*.html → new ✓
  7. kernel_meta/ → new ✓
  8. models/ → new ✓
  9. /geir/ → new ✓
  10. model_results/ → NEW: MISSING ✗
  11. **/*_result.csv → new ✓
  12. *.bin → new ✓
  13. *.zip → new ✓
  14. *.o → new ✓
  15. toml_data/ → new ✓
  16. model_*/ → new ✓
  17. models_*/ → new ✓
  18. lib/ → new ✓
  19. LLMAnalysisTool/ → new ✓
  20. settings.local.json → new ✓
  21. (blank)
  22. ttk/_build_hash.py → new ✓
  23. csrc/build/ → new ✓
  24. csrc/*/build/ → new ✓
  25. msprof/ → new ✓
  26. (blank)
  27. ttk/remote/server/ttk_xpu_sync/ → new ✓
  28. ttk/remote/server/ttk_tmp_dir/ → new ✓
  29. (blank)
  30. sim_output/ → new ✓
    ...
    Wait, old line 32 -sim_output/ and new line 22 +sim_output/ — ✓.

So only model_results/ dropped. This is the .gitignore regression.

Now let me evaluate severity of dropping model_results/: it's a directory the repo previously chose to ignore. Without it, if a model_results/ dir gets created during development, files could be accidentally committed. Minor (P3). I'll report it as P3.

Now, let me reconsider — should I also report that the whole "excel support" feature depends on a missing module as the only real issue? Let me also double-check the requirements.txt/pyproject for supply chain. openpyxl>=3.1 — legit package, no CVE concern. Fine.

Let me now also check ttk/utilities/__init__.py more carefully for the ruff ignore rationale. The per-file-ignores for F401/F403 in ttk/utilities/__init__.py — because the init now uses from .classes import * etc. That's the re-export pattern. Fine.

Actually, wait — one more thing about ttk/utilities/__init__.py: the diff REORDERED the imports. Previously the order was platform, classes, string_utils, container_utils, math, format_utils, file_utils, singleton, dtypes, plog_utils, proc, func_dispatch. Now: classes, container_utils, data (RandomData), dtypes, file_utils, format_utils, func_dispatch, math, platform, plog_utils, proc, singleton, string_utils, table_reader. Any circular import issues? from .classes import * first — classes.py imports... let me check classes.py imports: it imports numpy and TYPE_CHECKING testcase_op. Fine. from .func_dispatch import ... — func_dispatch imports? Might import from .data or .string_utils. If there's a circular import, it would have been caught already. Can't fully verify but the reorder is unlikely to introduce circular imports that weren't there. Skip.

Let me now also verify: are there any usages of resolved_sheet or read_table other than the ones I found? grep found the references. resolved_sheet is used in instance_base.py _resolved_output_sheet (line 496-498). It's expected to be exported from ttk.utilities via from .table_reader import *. Missing → P0.

Now let me finalize findings:

Finding 1 (P0): Missing ttk/utilities/table_reader.py module. All new imports (from .table_reader import * in ttk/utilities/__init__.py line 27; from ttk.utilities.table_reader import read_table in bridge.py line 344; from ...utilities.table_reader import read_csv_rows, read_table in testcase_manager.py line 23) reference a module that is neither in the diff nor on disk. Importing ttk.utilitiesModuleNotFoundError, breaking the entire tool. Anchor to ttk/utilities/__init__.py:27.

Confidence: 0.85. This is P0 (build/import failure).

Finding 2 (P3): .gitignore dropped model_results/ pattern during reorganization. Minor hygiene regression.

Let me also double-check: is there anything about sheet for .xlsm_load_cases handles .xlsm_load_case_from_tableread_table. And _resolved_output_sheet handles .xlsm. Fine.

Let me also reconsider the _open_result_file change for a potential subtle bug:
Old code for xlsx-like path: 'input.xlsx'.split(".") = ['input','xlsx']; split_input_path[-2] = 'input'; += '_result' → 'input_result'; join → 'input_result.xlsx'; not endswith .csv → += '.csv' → 'input_result.xlsx.csv'. That was the OLD behavior (weird but existing). New code → 'input_result.csv'. This is an improvement, not a regression.

But wait — there's a subtle NEW issue: _resolved_output_sheet is only about xlsx. For a plain csv input foo.csv → root="foo", sheet=None → result "foo_result.csv" — same as old. Good.

Now — is there any concern that _resolved_output_sheet could throw if the xlsx is unreadable (e.g. openpyxl error) and it's called before the actual load? It would raise at resolved_sheet — which would propagate as a crash with a confusing error rather than a fallback. But since the module's missing, moot. If the module existed and the file is corrupt, resolved_sheet might raise. But that's speculative about a missing module.

Let me now double-check the _load_cases early-return path: for .xlsx, _load_case_from_table is called and returns. But self.case_original_headers gets populated via list_append_union in _load_case_from_table. Then _prepare_output_titles uses self.case_original_headers. Fine.

Now, let me also examine the _detect_framework_from_csv for a real NEW bug: in the validation loop, detect_framework(row[api_idx]) is called WITHOUT checking row[api_idx] truthiness AND without .strip(). If read_table returns a row where api_idx < len(row) but row[api_idx] is "" (empty) — for a CSV with a trailing blank line, csv.reader returns [] (empty row) for a blank line, so api_idx < len(row) is False → skipped. For a row with only some columns... hmm. For an Excel sheet, openpyxl typically doesn't return blank trailing rows. So empty row[api_idx] with len(row) > api_idx happens when the api_name cell is genuinely empty but other cells are filled (e.g., a row with data in other columns). In that case, detect_framework("") is called.

The old code would ALSO call detect_framework("") in that case (it did row_api = row[api_idx].strip() then detect_framework(row_api) with no truthiness check). So equivalent. Not new.

But there IS a new difference: the OLD code used the FIRST data row (even if empty) as the framework baseline; the NEW code skips empty rows to find the first non-empty. Combined with the validation loop NOT skipping empty rows, we get this NEW false-positive scenario:

Old code, table: [first row: api empty, second row: tf.xxx, third row: torch.yyy]

  • Old: first_api="" → detect_framework("")="torch" → first_framework="torch"; row2 tf → mismatch → error.
  • New: first non-empty = row2 "tf.xxx" → first_framework="tf"; row3 torch → mismatch → error. Same outcome (error).

Scenario: [row1: tf.xxx, row2: empty api, row3: tf.yyy]

  • Old: first="tf"; row2 empty → detect_framework("")="torch" → mismatch → error.
  • New: first="tf" (row1); row2 empty → detect_framework("")="torch" → mismatch → error.
    Same.

So no NEW false-positive. The behavior is basically the same as old. OK, skip this.

Now let me consider whether the P0 missing-module finding should be split into the specific failure at testcase_manager.py — no, one root cause.

Let me reconsider whether there might be additional genuine issues:

  1. list_cmd.pyUniversalTestcaseFactory.from_path(args.input, args.sheet, skip_validate=True)from_path is a classmethod, called on the class. Fine. Note list_cmd previously read with open(args.input) — now from_path handles open/close. Fine (if module existed).

  2. common.py--sheet added to the common io args — used by run command. Fine.

  3. bridge.py _apply_io_args — fine.

Now, let me think about whether there's a --sheet propagation gap: run_with_switches for framework-api passes sheet to _detect_framework_from_csv. For geir and npu modes, sheet is not used in detection but is used in _load_cases (via switches.sheet). Fine.

Now, let me also double check SWITCHES pickle: __slots__ — pickling instance with slots: Python pickles slots via __reduce_ex__ (protocol 2+), which handles __slots__ attributes automatically using copyreg.__newobj__ and object.__setstate__. Actually, for classes with __slots__ and no __dict__, pickle protocol 2+ handles slot state. Should be fine. And sheet is a new slot set in __init__, so it gets pickled/unpickled correctly. Fine.

Now let me think about performance findings required by the task focus (CRITICAL performance). Hot-path: _detect_framework_from_csv reads the WHOLE table into memory (header, rows) — for large CSV files this loads everything just to detect framework, whereas old code streamed with csv.reader. This is a memory/performance regression for large CSV input: previously only iterated rows lazily; now read_table materializes all rows. For huge CSV test case files this could be significant. But it's one-time at startup, not a hot loop. Given "CRITICAL performance" focus, this might be worth a P3 note: _detect_framework_from_csv now materializes the entire table (rows) into memory just to check api_name consistency, whereas the old code streamed rows lazily. For very large inputs this increases peak memory. Hmm — but it's a one-time cost at startup, bounded by the file size, and the file will be loaded again anyway for testcase parsing. The added memory is transient. I think this is borderline noise. I'll consider reporting as P3 low confidence — actually the review guidelines say report concrete performance regressions at P3 even if impact uncertain. This is a concrete change: full-table materialization in _detect_framework_from_csv. But honestly it also reads it lazily in the second loop... no, read_table returns a materialized list of rows. I'll report it as P3, low confidence.

Hmm, actually let me reconsider. The whole feature depends on the missing module. Reporting sub-issues of a feature that can't even import is somewhat moot, but each is still a real issue in the diff. The performance one is defensible.

Let me also double check one more potential correctness issue: in testcase_manager.py, __init__(self, file: TextIO, skip_validate=False) calls read_csv_rows(file). Previously _read_csv(file) used csv.reader(file) and stripped each cell. New read_csv_rows — can't verify behavior. Since module missing, no.

OK now, let me also look at whether raw_data rows are now NOT stripped. In _init_from_rows: self.raw_data = [list(row) for row in rows]. If read_table/read_csv_rows return unstripped values, then _testcase_hdr_check checks "testcase_name" in self.header and "api_name" in self.header — if header cells have trailing whitespace (e.g. "api_name " with trailing space), the checks fail → wrong testcase_instance selection or appended auto names. The old _read_csv explicitly stripped each cell. This stripping responsibility moved to read_csv_rows/read_table. If those don't strip, it's a regression. But we can't see the implementation. Since the module is missing entirely, this is moot / speculative.

I'll keep findings to:

  1. P0: missing table_reader module.
  2. P3: .gitignore dropped model_results/.
  3. Maybe P3: _detect_framework_from_csv materializes whole table (performance/memory).

Let me reconsider #3 — is it really worth reporting? The guidelines for performance: "per-call allocations, repeated compilation/loading, and lock scope inside frequently-invoked methods — report concrete ones at P3 even when impact is uncertain (lower confidence, do not drop)." _detect_framework_from_csv is called once per run (startup). It's not "frequently-inv

类型 数量
🔴 阻塞 1
🟡 建议 0

⛔ 需要修改

likedislike
不准确?
CANN-robotCANN-robot成员
15 天前 添加了label:cann-cla/yes
CANN-robot
CANN-robot成员
15 天前 评论:

CLA Signature Pass

wangrui_, thanks for your pull request. All authors of the commits have signed the CLA. 👍

likedislike
此处折叠了50条消息 查看更多
CANN-robotCANN-robot成员
14 天前 添加了label:approved
sunday成员
14 天前 评论:

/lgtm
/approve

likedislike
CANN-robotCANN-robot成员
14 天前 添加了label:lgtm
CANN-robotCANN-robot成员
14 天前 关闭了关联的issue
CANN-robotCANN-robot成员
14 天前 合入了pull request