修复 datasets.py 与 HuggingFace datasets 库的命名冲突

[joyehuang]

📝 变更描述

修复 modules/common/datasets.py 与 HuggingFace datasets 库的命名冲突问题。将文件重命名为 data_sources.py，并同步更新所有相关文档中的引用。

问题背景：

• 当前文件名 datasets.py 与 HuggingFace 的 datasets 库同名
• 导致 Python 导入时优先导入本地文件而非第三方库
• 造成 TinyStories 数据集下载失败（from datasets import load_dataset 报错）

解决方案：

• 将 datasets.py 重命名为 data_sources.py
• 更新所有文档中的命令示例和导入语句
• 包括中英文文档的同步更新

🎯 变更类型

• 🐛 Bug 修复
• ✨ 新功能
• 📖 文档更新
• 🔬 新实验
• 🎨 代码风格/格式化（不影响功能）
• ♻️ 代码重构
• ⚡ 性能优化
• ✅ 测试相关
• 🔧 构建/工具相关
• 🌍 翻译

📚 相关模块

• 01-normalization
• 02-position-encoding
• 03-attention
• 04-feedforward
• 02-architecture
• 文档/网站
• 通用工具
• 其他: ___________

🔗 相关 Issue

• Closes [BUG] python datasets.py --download-all这里的py文件和datasets库的名字冲突了 #19

✅ 检查清单

代码质量

• 代码遵循项目的代码风格
• 代码有充分的中文注释
• 没有引入新的 linter 错误
• 代码可以独立运行（如果是实验代码）

文档相关（如果适用）

• 更新了相关 README
• 添加了必要的代码注释
• 检查了拼写和语法

测试

• 我已经测试了这些更改
• 没有破坏现有功能

📋 修改文件清单

核心文件:

• modules/common/datasets.py → modules/common/data_sources.py

中文文档 (6个):

• modules/README.md
• modules/index.md
• modules/01-foundation/README.md
• docs/guide/quick-start.md
• ROADMAP.md
• modules/common/data_sources.py (内部文档)

英文文档 (4个):

• en/modules/index.md
• en/modules/01-foundation/index.md
• en/docs/guide/quick-start.md
• en/ROADMAP.md

🔍 测试说明

测试这个修复的步骤：

1. 克隆分支并安装依赖

   git checkout fix/issue-19-datasets-naming-conflict
   pip install datasets requests

2. 测试数据下载功能

   cd modules/common
   python data_sources.py --download-all

3. 预期结果

   • ✅ TinyShakespeare 下载成功
   • ✅ TinyStories 10MB 下载成功（不再报错 cannot import name 'load_dataset'）
   • ✅ TinyStories 50MB 下载成功

4. 验证导入功能

   from modules.common.data_sources import get_experiment_data
   text = get_experiment_data('shakespeare')
   print(f"加载成功: {len(text)} 字符")

📝 额外说明

• 此修复向后不兼容：如果有代码使用 from modules.common.datasets import ...，需要更新为 from modules.common.data_sources import ...
• 所有官方文档已同步更新，用户按照新文档操作即可
• 文件重命名使用 git mv 保留了 Git 历史记录

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
minimind-notes	Ready	Preview, Comment	Feb 21, 2026 7:54am

[claude]

PR Review: 修复 datasets.py 命名冲突

总体评价: 这是一个清晰、目标明确的 bug 修复。问题根因分析准确，解决方案简洁有效。

---

正面评价

根因分析准确

命名冲突的原因是 Python 模块搜索路径在解析 from datasets import load_dataset 时，当前工作目录（modules/common/）优先于 site-packages，导致本地 datasets.py 遮蔽了 HuggingFace 库。重命名为 data_sources.py 是最简洁的修复方式，优于在函数内部使用 importlib 等绕过方法。

文档覆盖全面

中英文文档（10 个文件）全部同步更新，包括 README、快速入门、ROADMAP，细节到位。

Python 文件无遗漏

经过检查，仓库中没有其他 Python 文件从 modules.common.datasets 导入内容，也没有 __init__.py 需要同步更新，PR 变更范围是完整的。

保留 Git 历史

使用 git mv 重命名文件，Git 历史连续性得以保留。

---

需要关注的问题

1. 现有实验代码的兼容性（低风险，但值得确认）

目前搜索结果显示没有实验 .py 文件使用 get_experiment_data，但随着后续模块加入，新实验代码如果使用旧文件名会出错。建议在 modules/common/ 下加一条注释或文档提示说明旧名称已废弃，方便未来贡献者排查。

2. data_sources.py 内部文档字符串

顶部的示例 import 已更新，很好。如果文件内还有其他引用 datasets.py 文件名的注释，需确认已一并更新。从 diff 看只有 1 行变更，应已覆盖。

3. 向后不兼容说明位置

PR 描述中写了"此修复向后不兼容"，但这个信息只在 PR body 里，合并后没有在文档中体现。可以考虑在相关文档中加一行迁移说明，方便直接克隆仓库的用户了解变更。

---

小建议（可选）

在 data_sources.py 顶部加一行注释，说明命名的原因，方便未来维护者理解为什么不叫 datasets.py（避免有人将来"好心"重命名回去）。

---

结论

建议合并（Approve）。修复方向正确，文档覆盖完整，无遗漏的 Python 导入。上面几点都是改进建议，不是阻塞项。

[claude]

PR Review: 修复 datasets.py 命名冲突

总体评价：这是一个正确且必要的修复。命名冲突问题定位准确，文档更新全面（中英文同步）。以下是具体反馈。

---

✅ 优点

• 问题定位准确：Python 的模块搜索路径会优先加载本地 datasets.py，确实会导致 from datasets import load_dataset 失败
• 旧文件保留并给出清晰错误信息，比直接删除更友好
• 文档更新覆盖全面（6个中文 + 4个英文文档）
• 新增 modules/common/README.md 是个好补充

---

🔴 需要修复的问题

1. Python 版本不兼容（data_sources.py:33）

# 当前代码 - 仅支持 Python 3.10+
def get_experiment_data(...) -> str | List[str]:

str | List[str] 语法要求 Python 3.10+。项目没有声明最低版本要求，建议改为兼容写法：

from typing import Union

def get_experiment_data(...) -> Union[str, List[str]]:

---

🟡 建议改进

2. datasets.py 的处理方式可以更友好

当前方案在模块级别直接 raise ImportError，会导致 pytest / mypy / 代码扫描工具在自动发现文件时报错。

更推荐的模式是发出废弃警告同时保持向后兼容：

import warnings
warnings.warn(
    "datasets.py 已重命名为 data_sources.py，请更新导入语句。",
    DeprecationWarning,
    stacklevel=2
)
from modules.common.data_sources import *  # noqa: F401, F403

这样现有代码不会立即崩溃，用户可以在不紧急的情况下迁移。如果确实想强制迁移（破坏性变更），当前的 raise 方案也可以接受，但需要在 PR 描述中明确说明。

3. 模块级副作用（data_sources.py:25）

DATA_DIR.mkdir(exist_ok=True)  # 模块级别执行

在模块 import 时创建目录属于隐式副作用。建议移到实际需要写入时再创建（如 _get_shakespeare 函数内），或保留此方式但加一行注释说明意图。

4. 迁移说明重复

modules/README.md 和 modules/index.md 都添加了相同的迁移说明块，内容完全重复。如果两个文件长期并行维护，未来可能出现内容漂移。可以考虑在一个文件中维护权威信息，另一个引用即可。

---

ℹ️ 小问题

• _generate_synthetic 函数接受 size_mb 参数但未接受 cache 参数，与其他私有函数签名不一致（这是老代码问题，不是本 PR 引入的）
• requests.get 没有重试逻辑，网络抖动时用户体验较差，可以后续改进

---

总结

项目	评价
问题修复正确性	✅
文档完整性	✅
Python 兼容性	🔴 需修复
向后兼容策略	🟡 可商榷

建议：修复 Python 3.10+ 的类型注解兼容性问题后，此 PR 可以合并。

[claude]

代码审查意见

感谢提交这个 PR！整体方向正确，以下是详细的审查意见。

---

总体评价

✅ 正确定位了根本原因：PR 准确识别出 datasets.py 命名冲突导致的 ImportError 问题。
✅ 文档更新全面：同时更新了中英文文档（共约 10 个文件），覆盖面广。
✅ 新增 modules/common/README.md：对模块目录新增说明文档，是一个很好的补充。
✅ 新增系统要求说明：在文档中补充了环境依赖说明，对用户友好。
✅ 详细的测试步骤：PR 描述中附带了完整的测试指令，便于验证。

---

⚠️ 关键问题（建议合并前修复）

datasets.py 保留为 stub 并未彻底解决问题

当前方案将 datasets.py 改写为在模块级别抛出 ImportError 的 stub，但这并不能完全修复原始问题。

问题场景：当用户在 modules/common/ 目录下运行脚本时，Python 会将当前目录 . 插入 sys.path[0]。此时 data_sources.py 执行：

from datasets import load_dataset

Python 的模块解析顺序会优先找到本地的 datasets.py，触发 stub 中的 ImportError，与原来的问题完全相同。

验证方法：

cd modules/common
python -c "import data_sources"
# 仍然会触发 ImportError

根本原因：只要本地目录存在 datasets.py，无论内容是什么，都会在路径优先级上遮蔽 PyPI 的 datasets 包。

建议修复方案：直接删除 datasets.py，git 历史通过 git log -- datasets.py 仍可追溯：

git rm modules/common/datasets.py
git commit -m "移除与 PyPI datasets 包冲突的同名文件"

保留 stub 的唯一合理场景是故意阻止导入，但此处显然不是这个意图。

---

💡 次要建议（可选，不阻塞合并）

1. 检查 modules/common/__init__.py 的导出

如果 modules/common/__init__.py 中有类似以下内容：

from .datasets import ...

则删除 datasets.py 后需要同步更新，否则会导致新的导入错误。建议确认。

2. 重复的迁移说明

modules/index.md 和 modules/README.md 中均包含相同的迁移提示内容，后续维护时需要同步两处，增加了维护负担。可以考虑在一处维护，另一处引用。

3. visualization.py 描述不够明确

README 中对 visualization.py 的描述使用了"if exists"等模糊措辞，建议明确说明该文件的实际状态（是可选依赖还是必须存在）。

4. TinyStories 缓存文件名问题

若 size_mb 使用浮点数，可能生成如下文件名：

tinystories_10.5mb.json

浮点数文件名在不同平台和脚本中可能引发路径解析问题。建议改为整数或字符串格式化：

filename = f"tinystories_{int(size_mb)}mb.json"

---

结论

这个 PR 的分析思路清晰，文档更新工作量充分，是一次高质量的贡献。建议在解决上述关键问题（删除 datasets.py）后即可合并。其余次要建议可作为后续优化跟进。

再次感谢你的工作！

[claude]

PR Review: 修复 datasets.py 命名冲突

总体评价：这个 PR 解决了一个真实存在的 Bug，根本原因分析准确，文档更新全面（中英双语同步），代码改动本身也有几处额外的小优化。以下是具体反馈。

---

✅ 做得好的地方

1. 根本原因分析正确：Python 模块搜索路径确实优先当前目录，datasets.py 与 HuggingFace 库同名会导致 ImportError，重命名是正确的解决方案。

2. 惰性目录创建：将 DATA_DIR.mkdir() 从模块级移到实际需要时才创建，这是一个真正的改进——避免了在纯导入时就在文件系统写数据。

3. 浮点数文件名修复：

   # 旧代码
   cache_file = DATA_DIR / f'tinystories_{size_mb}mb.json'  # 可能生成 tinystories_10.0mb.json
   # 新代码
   cache_file = DATA_DIR / f'tinystories_{int(size_mb)}mb.json'  # 更干净

   这个修复有意义，但见下面的注意事项。

4. 文档覆盖面好：中英文文档同步更新，迁移说明清晰。

---

⚠️ 需要关注的问题

问题 1：README 描述与实际实现不符（文档 Bug）

modules/common/README.md 中写道：

为了帮助排查问题，我们保留了 datasets.py 文件，但它会抛出清晰的错误提示

但从 diff 来看，datasets.py 是通过 git mv 直接重命名的，原文件已不存在。用户使用旧导入方式时会收到标准的 ModuleNotFoundError: No module named 'modules.common.datasets'，而不是"友好的错误提示"。

建议：两种方案二选一：

• 删除 README 中这句话，如实说明旧文件已删除；
• 真正创建一个 datasets.py 兼容垫片：

  # modules/common/datasets.py (compatibility shim)
  raise ImportError(
      "'modules.common.datasets' 已重命名为 'modules.common.data_sources'。\n"
      "请将导入更新为: from modules.common.data_sources import get_experiment_data"
  )

问题 2：int() 截断对非整数 size_mb 会静默丢失信息

cache_file = DATA_DIR / f'tinystories_{int(size_mb)}mb.json'

如果调用者传入 size_mb=10.5，缓存文件名会变成 tinystories_10mb.json，与 size_mb=10 的缓存混用，可能返回数据量不足的缓存。

建议：如果 API 只打算支持整数 MB，在函数签名中直接声明：

def _get_tinystories(size_mb: int, cache: bool = True) -> List[str]:

或在函数入口加断言：

assert size_mb == int(size_mb), f"size_mb 必须为整数，got {size_mb}"

问题 3：migration notice 中的日期与当前年份不符

多处文档写 "2025-02"，但实际日期是 2026 年 2 月。这是笔误，建议改为 2026-02。

问题 4：文档中硬编码 PR URL（低优先级）

en/modules/index.md 和 modules/README.md 中有：

详见 PR #20: #20

PR 链接作为长期文档的引用略显奇怪，建议改为指向迁移说明的内部链接（如 modules/common/README.md），信息更自包含。

---

📋 次要建议

• modules/common/README.md 是新增文件，内容质量很好，但 visualization.py 那节只有"如果存在"这样模糊的描述，建议补充是否该文件在项目中实际存在。
• 系统要求中 "最低 Python 3.10" 和 "推荐 Python 3.10+" 重复，可简化为一条说明。

---

总结

方面	评价
根本原因修复	✅ 正确
代码质量	✅ 有改进
文档覆盖	✅ 全面
README 与实现一致性	❌ 需修复
年份笔误	⚠️ 建议修复

核心 Bug 修复没有问题，建议在合并前处理「README 描述与实现不符」这个点——要么真正创建兼容垫片，要么修改文档说明。其余问题为次要建议。

[claude]

PR Review: Fix datasets.py Naming Conflict

Overall this is a clean, well-executed fix. The root cause is correctly identified and the solution is appropriate. Below are my observations.

---

✅ Strengths

Correct diagnosis and fix: Python's module search order (current directory before site-packages) means a local datasets.py shadows the HuggingFace library. Renaming to data_sources.py is the right approach.

Lazy directory creation (good improvement):

# Before (creates dir at import time — bad practice)
DATA_DIR.mkdir(exist_ok=True)

# After (creates dir only when actually needed — better)
DATA_DIR.mkdir(exist_ok=True)  # inside _get_shakespeare() and _get_tinystories()

This avoids side effects at import time and is a genuine improvement over the original code.

Documentation coverage: All 10 affected files (Chinese + English) are updated consistently. The migration notice in modules/common/README.md is clear and includes a before/after comparison table.

Inline explanation in data_sources.py: The file-level docstring explains exactly why the name is what it is — this is valuable for future maintainers who might otherwise be tempted to rename it back.

---

🔍 Issues and Suggestions

1. Type annotation regression: float → int for size_mb

# Before
def get_experiment_data(size_mb: Optional[float] = None, ...) -> str | List[str]:

# After
def get_experiment_data(size_mb: Optional[int] = None, ...) -> str | List[str]:

Changing float to int is a breaking API change. If a caller passes size_mb=10.5, Python won't raise an error (type hints aren't enforced at runtime), but the annotation now misleads. Since the parameter represents a size limit used for iteration, float is the more permissive and correct type. If only integers are actually meaningful here, add a runtime check instead:

if size_mb is not None and not isinstance(size_mb, int):
    raise TypeError(f"size_mb must be an integer, got {type(size_mb).__name__}")

2. visualization.py is documented but doesn't exist

modules/common/README.md documents visualization.py with:

注意: 此文件目前尚未创建，计划在后续模块中添加。

Documenting a non-existent file in a README is confusing for new users who might try to import it. Consider either removing this section until the file exists, or moving it to a "Planned" section clearly separated from available tools.

3. Migration notice is duplicated across 4 locations

The migration notice appears in:

• modules/README.md
• modules/index.md
• en/modules/index.md
• modules/common/README.md

This is thorough now, but creates a maintenance burden — if any detail changes later, all 4 need updating. Consider keeping the canonical explanation only in modules/common/README.md and having the others link to it (which some already do).

4. No compatibility shim (minor, acceptable)

Since datasets.py was already broken (it caused the import error), there's no practical benefit to a compatibility shim. This is explicitly called out in the PR description and is fine given the educational nature of this repo.

---

📋 Summary

Category	Assessment
Core fix	✅ Correct and appropriate
Code quality	✅ Improved (lazy mkdir)
Documentation	✅ Comprehensive
Breaking change handling	✅ Clearly communicated
Type annotation	⚠️ float→int change is unnecessary and misleading
Non-existent file docs	⚠️ Could confuse new users

The float→int type annotation issue is the main thing worth addressing before merging. The non-existent visualization.py documentation is a minor concern. Everything else looks good.