From 6758e81c9dc670206aabca40ffd2479cbbc9e4bd Mon Sep 17 00:00:00 2001 From: guzhujushi Date: Thu, 1 Oct 2026 23:26:47 +0800 Subject: [PATCH] =?UTF-8?q?fix(data):=20=E6=A0=A1=E9=AA=8C=E6=97=A9?= =?UTF-8?q?=E4=BA=8E=E5=89=AF=E4=BD=9C=E7=94=A8=EF=BC=8C=E8=A1=A5=E7=A9=BA?= =?UTF-8?q?=E8=A1=8C=EF=BC=9B=E7=BB=8F=E9=AA=8C=E5=86=99=E5=85=A5=20pitfal?= =?UTF-8?q?ls?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit save_table 的两处收尾（用户授权代改）： - 后缀校验原先藏在 if/else 分支的 else 里，排在 path.parent.mkdir(...) 之后， 传 sub/x.txt 会先把 sub\ 建出来再抛 ValueError。改成守卫子句，集中在 mkdir 之前， 校验段结束后才产生副作用；此时 else 分支自然消失，用回 if/else 两路分派 - __all__ 与下一个 def 之间补回第二个空行（PEP 8 / E302） 测试： - 新增 test_save_table_validates_before_touching_disk， 断言参数非法时 tmp_path 下一个条目都不留（ValueError 与 TypeError 两条路径） - 40 passed / 44 skipped，两套环境一致；ruff All checks passed 文档： - docs/dev/personal-pitfalls.md 追加条目 5（参数校验排在副作用之后） 与条目 6（把 lint 的"没报错"当成"没问题"），索引表同步加行 - STATUS：io.py 收尾，L0 第 3 项完成；测试计数 40 - CHANGELOG：Fixed 段补两条 --- CHANGELOG.md | 3 +++ STATUS.md | 14 ++++++++------ docs/dev/personal-pitfalls.md | 32 ++++++++++++++++++++++++++++++++ src/mcm_toolkit/data/io.py | 9 ++++++--- tests/test_data.py | 12 ++++++++++++ 5 files changed, 61 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fee1cb7..79806b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,9 @@ - `docs/dev/personal-pitfalls.md`：个人易错点清单，**只追加、不通读** ### Fixed +- `data.save_table` 的后缀校验排在 `path.parent.mkdir(...)` **之后**，传 `.txt` 会先留下一个 + 空目录再抛 `ValueError`。改为**先校验、后副作用**，并加回归测试。 +- `data/io.py` 里 `__all__` 与下一个 `def` 之间只有 1 个空行（PEP 8 要 2 个）。 - `tests/test_data.py` 顶上的**模块级** `pytestmark = pytest.mark.skip(...)` 会连带跳过同文件里 已经写好的函数，导致 `load_table` 零覆盖。改为逐条 skip 未实现的算法，并补 8 条 io 测试 （编码探测、NA 归一、后缀校验、缺文件报错、往返读写、类型校验）。测试数 31 → **39 passed**。 diff --git a/STATUS.md b/STATUS.md index a417c33..b56b582 100644 --- a/STATUS.md +++ b/STATUS.md @@ -85,7 +85,9 @@ utf-8 → utf-8-sig → gbk → gb18030，显式传 `encoding=` 时不再探测 - `na_values` 默认吃下空串 / `NA` / `N/A` / `#N/A` / `null` / `-` - 不支持的扩展名抛 `ValueError`；文件不存在抛带路径的 `FileNotFoundError` - - 测试：`tests/test_data.py` 由 1 条（且被模块级 skip 掉）扩到 **8 条** + - `save_table` 的校验顺序已修正：后缀判断提到 `mkdir` 之前 + （此前传 `.txt` 会先留下一个空目录再抛错） + - 测试：`tests/test_data.py` 由 1 条（且被模块级 skip 掉）扩到 **9 条** - docstring 的 `Parameters` / `Raises` 已按 §4.7 补齐（Codex 代写，用户授权）—— numpydoc 格式，对齐参考实现 `core/random.py` 的 `get_rng` - [ ] 剩余 9 项（10 个函数）：`clean_missing` @@ -96,7 +98,7 @@ | 检查 | venv | conda `mcm-toolkit` | |---|---|---| -| `pytest` | **39 passed, 44 skipped** | **39 passed, 44 skipped** | +| `pytest` | **40 passed, 44 skipped** | **40 passed, 44 skipped** | | `import mcm_toolkit` | 0.0.1 / 16 项 | 0.0.1 / 16 项 | | `ruff check` | All checks passed | All checks passed | | `scripts/doctor.ps1` | 跑通 | — | @@ -105,7 +107,7 @@ - base 亦已复核：numpy 2.4.6 / scipy 1.18.1 / pandas 3.0.6（与两套环境一致） - skip 的都是等实现的算法行为测试；通过的 39 条 = 20 条公开 API 契约测试 （`tests/test_api_surface.py`）+ 11 条 `core` 行为测试（`tests/test_core.py`） - + 8 条 `data` 行为测试（`tests/test_data.py`） + + 9 条 `data` 行为测试（`tests/test_data.py`） - 三个 `.ps1` 通过 PowerShell AST 语法解析 ### ⚠️ 需要注意 / 遗留 @@ -162,9 +164,9 @@ ## 下一步 -1. **`data/io.py` 还剩一处**：`isinstance(df, pd.DataFrame)` 已经挪到 `mkdir` 前面了， - 但**后缀校验还留在 `else` 分支里**，仍在 `mkdir` 之后。传 `sub/x.txt` 依旧会先建出 - `sub/` 再抛 `ValueError`（实测确认）。把后缀判断也提到 `mkdir` 前面即可。 +1. ~~`save_table` 校验顺序~~ **已修**：后缀判断提到 `mkdir` 前面，并补了回归测试 + `test_save_table_validates_before_touching_disk`（断言参数非法时 `tmp_path` 下一个条目都不留）。 + `io.py` 至此收尾，L0 的第 3 项完成。 2. 继续 L0，**下一个：`data/clean.py` 的 `clean_missing`**（L0 / A 档，直接能写）。 同文件的 `detect_outliers` 是 **S 档**，要先按 `docs/dev/function-card-template.md` 把函数卡片写出来再动手。 diff --git a/docs/dev/personal-pitfalls.md b/docs/dev/personal-pitfalls.md index df62861..bdd7525 100644 --- a/docs/dev/personal-pitfalls.md +++ b/docs/dev/personal-pitfalls.md @@ -26,6 +26,8 @@ | 2 | 以为 `# TODO(L0)` 只是普通注释 | 2026-10-01 | | 3 | 签名里的类型注解和 docstring 说的不一致 | 2026-10-01 | | 4 | 只覆盖自己的状态，忘了第三方库还在用 numpy 旧全局 | 2026-10-01 | +| 5 | 参数校验排在副作用（建目录 / 写文件）之后 | 2026-10-01 | +| 6 | 把 lint 的"没报错"当成"没问题" | 2026-10-01 | --- @@ -66,3 +68,33 @@ - 以后怎么防：凡是"设全局状态"的函数，先列清单——自己的、numpy 旧的、标准库 `random` 的、 第三方框架的，逐个决定要不要一起设，并在 docstring 里写清到底覆盖了哪几个。 - 首次：2026-10-01｜复发：0 + + +### 5. 参数校验排在副作用（建目录 / 写文件）之后 + +- 现象：`save_table` 先 `path.parent.mkdir(parents=True, exist_ok=True)`，再在 + `if suffix == '.csv' ... else: raise ValueError` 里判断后缀。传 `sub/x.txt` 时， + 空目录 `sub\` 已经建出来了，函数才抛 `ValueError`——错误路径上留下垃圾。 +- 为什么会错：函数体是照着"顺利流程"顺着写下来的——先建目录、再按分支干活； + 校验顺手塞进分支的 `else` 里，于是校验自动排到了最后。分支越多，越会有路径漏掉。 + 会传染：`isinstance` 校验改到 `mkdir` 前面之后，后缀校验**还是**留在原地， + 因为它藏在分支里，看起来"已经检查过了"。 +- 以后怎么防：函数体分三段写——**① 所有校验 → ② 所有副作用（建目录 / 写文件 / + 联网 / 改全局状态）→ ③ 主逻辑**。校验用守卫子句集中在开头，不要塞进分支里； + 凡是 `raise` 的行，位置必须在校验段内，不能出现在副作用段之后。 + 写完自问一句："这个参数不合法时，磁盘 / 网络 / 全局状态会不会已经动过了？" +- 首次：2026-10-01｜复发：0 + +### 6. 把 lint 的"没报错"当成"没问题" + +- 现象：`__all__` 与下一个 `def` 之间只留了 1 个空行（PEP 8 要求 2 个）， + `ruff check src tests` 照样打印 "All checks passed!"。 +- 为什么会错：版本号看着一样，覆盖的规则却是两回事。ruff 0.16 里 `E302` + （函数间空行）属于 **preview 规则**，不在 `select = ["E"]` 的默认集合里—— + 加了也会被"because preview is not enabled"忽略掉，安安静静不报。 + 根本问题是：**lint 只检查你 select 的那部分**，而默认规则集本来就不是 PEP 8 全集。 + 把"我配的检查全过了"读成"这份代码符合规范"，是把自己没开的检查当成了已经通过的检查。 +- 以后怎么防：把 lint 结论限定成"我 select 的规则里没问题"，别说成"没问题"。 + 想看全貌就偶尔跑一次 `ruff check --select ALL --preview`（只当参考，别直接开进 CI， + 会淹没在噪声里）；纯格式问题交给 `ruff format`，它是 formatter，不受 `select` 影响。 +- 首次：2026-10-01｜复发：0 diff --git a/src/mcm_toolkit/data/io.py b/src/mcm_toolkit/data/io.py index 36b764a..a8ed4f3 100644 --- a/src/mcm_toolkit/data/io.py +++ b/src/mcm_toolkit/data/io.py @@ -14,6 +14,7 @@ import pandas as pd __all__ = ["load_table", "save_table"] + def load_table(path: str | Path, **kwargs: Any) -> pd.DataFrame: """按扩展名自动选择读取方式，返回 DataFrame。 @@ -100,12 +101,14 @@ def save_table(df: pd.DataFrame, path: str | Path, index: bool = False, **kwargs if not isinstance(df,pd.DataFrame): raise TypeError(f"df 必须是一个pandas DataFrame,但是你上传了{type(df)}") + # 校验必须排在副作用（建目录）之前：否则传个 .txt 会先建出一个空目录再抛错。 + if suffix not in ('.csv','.xlsx','.xls'): + raise ValueError(f"不支持的文件后缀：{suffix}") + path.parent.mkdir(parents=True,exist_ok=True) if suffix == '.csv': df.to_csv(path,index=index,**kwargs) - elif suffix in ['.xlsx','.xls']: - df.to_excel(path,index=index,**kwargs) else: - raise ValueError(f"不支持的文件后缀：{suffix}") + df.to_excel(path,index=index,**kwargs) return path diff --git a/tests/test_data.py b/tests/test_data.py index 09384b8..7d476b0 100644 --- a/tests/test_data.py +++ b/tests/test_data.py @@ -105,6 +105,18 @@ def test_save_table_rejects_non_dataframe(tmp_path): save_table([1, 2, 3], tmp_path / "bad.csv") +def test_save_table_validates_before_touching_disk(tmp_path): + """校验必须早于副作用：参数不合法时，一个目录、一个文件都不能留下。""" + from mcm_toolkit.data import save_table + + with pytest.raises(ValueError, match=r"\.txt"): + save_table(pd.DataFrame({"a": [1]}), tmp_path / "d1" / "x.txt") + with pytest.raises(TypeError): + save_table([1, 2, 3], tmp_path / "d2" / "x.csv") + + assert sorted(p.name for p in tmp_path.iterdir()) == [] + + # ------------------------------------------------- 以下为未实现算法（L0/L1 待写）