审核人指南#
审核开放的 pull request (PR) 有助于推动项目向前发展.我们鼓励项目外部的人员也参与进来;这是熟悉代码库的好方法.
谁可以成为审核人?#
评论可以来自 NumPy 团队之外 - 我们欢迎领域专家(例如, linalg 或 fft )或其他项目的维护者的贡献.您无需成为 NumPy 维护者(具有合并 PR 权限的 NumPy 团队成员)即可进行审查.
如果我们还不认识您,请考虑在您开始审查 pull request 之前在 the mailing list or Slack 中介绍一下自己.
沟通指南#
每个 PR,无论好坏,都是一种慷慨的行为.以积极的评论开头将有助于作者感到受到奖励,并且您的后续评论可能会被更清楚地听到.你也可能会感觉很好.
如果可能,从大问题开始,以便作者知道他们已被理解.抵制住立即逐行浏览或从小的普遍性问题开始的诱惑.
你是这个项目的形象代表,NumPy 不久前决定了 the kind of project it will be :开放,善解人意,热情,友好和耐心.对贡献者要 kind .
不要让完美成为优秀的敌人,特别是对于文档.如果您发现自己提出了许多小建议,或者对风格或语法过于吹毛求疵,请考虑在所有重要问题都得到解决后合并当前的 PR.然后,直接推送提交(如果您是维护者)或自己打开后续 PR.
如果您在撰写评论回复时需要帮助,请查看一些 standard replies for reviewing .
审核人清单#
- 在所有情况下,预期行为是否清晰? 一些需要注意的事情:
对于空数组或 nan/inf 值等意外输入会发生什么?
是否测试过 axis 或 shape 参数为
int或tuples?如果函数支持非常规的
dtypes,是否对这些dtypes进行了测试?
变量名是否应改进以提高清晰度或一致性?
应该添加注释,还是应该删除不helpful或无关的注释?
文档是否符合 NumPy guidelines ?文档字符串的格式是否正确?
代码是否符合 NumPy 的 Stylistic Guidelines ?
如果您是维护者,并且从 PR 描述中看不明显,请在合并消息中添加一个简短的解释说明分支做了什么,如果关闭了一个 issue,还要添加“Closes gh-123”,其中 123 是 issue 编号.
对于代码更改,至少需要一位维护者(即具有提交权限的人)审查并批准 pull request.如果您是第一个审查 PR 并批准更改的人,请使用 GitHub 的 approve review 工具将其标记为已批准.如果 PR 很简单,例如,它是一个明显正确的错误修复,则可以立即合并.如果它更复杂或更改了公共 API,请至少保留几天,以便其他维护者有机会进行审查.
如果您是已批准的 PR 的后续审阅者,请使用与新 PR 相同的审阅方法(关注更大的问题,抵制只添加一些小问题的诱惑).如果您有提交权限并认为不再需要审查,请合并 PR.
对于维护者#
在合并 PR 之前,请确保所有自动 CI 测试都通过,并且 documentation builds 没有任何错误.
如果出现合并冲突,请要求 PR 提交者 rebase on main .
对于添加新功能或在某些方面很复杂的 PR,请至少等待一两天后再合并.这样,其他人就有机会在代码进入之前发表评论.考虑将其添加到发行说明中.
在合并贡献时,提交者有责任确保这些贡献满足 NumPy 的 Development process guidelines 中概述的要求. 此外,检查是否在 numpy-discussion mailing list 上讨论过新功能和向后兼容性中断.
如果您认为 PR 太混乱,可以合并提交或清理提交消息.记住在这样做时保留原始作者的姓名.确保提交消息符合 rules for NumPy .
当您要拒绝 PR 时:如果它非常明显,您可以直接关闭它并解释原因.如果不是,那么最好先解释您为什么认为 PR 不适合包含在 NumPy 中,然后让第二个提交者评论或关闭.
如果 PR 提交者在 6 个月内未回复您的评论,请使用“inactive”标签将相关 PR 移至非活动类别. 此时,PR 可以由维护者关闭. 如果有兴趣完成正在考虑的 PR,则可以随时通过评论表明,无需等待 6 个月.
鼓励维护者在合并之前仅需要进行少量更改时(例如,修复代码样式或语法错误)完成 PR. 如果 PR 变为非活动状态,则维护者可能会进行更大的更改. 请记住,PR 是贡献者和审阅者之间的协作,有时直接推送是完成它的最佳方式.
API 变更#
如前所述,大多数公共 API 的更改应提前讨论,并经常与更广泛的受众讨论(在邮件列表中,甚至通过 NEP).
对于公共 C-API 的更改,请注意 NumPy C-API 是向后兼容的,因此任何添加都必须与以前的版本 ABI 兼容.如果不是这种情况,您必须添加一个保护.
例如, PyUnicodeScalarObject 结构包含以下内容:
#if NPY_FEATURE_VERSION >= NPY_1_20_API_VERSION
char *buffer_fmt;
#endif
因为 buffer_fmt 字段在 NumPy 1.20 中被添加到它的末尾(所有之前的字段都保持 ABI 兼容). 类似地,添加到 numpy/_core/code_generators/numpy_api.py 中的 API 表中的任何函数必须使用 MinVersion 注释. 例如:
'PyDataMem_SetHandler': (304, MinVersion("1.22")),
仅标头功能(例如,新的宏)通常不需要保护.
GitHub 工作流程#
在审查 pull request 时,请酌情使用 GitHub 上的工作流程跟踪功能:
完成审查后,如果您想要求提交者进行更改,请将您的审查状态更改为“请求更改”. 这可以在 GitHub,PR 页面,“文件已更改”选项卡,“审查已更改”(右上角的按钮)上完成.
如果您对当前状态感到满意,请将 pull request 标记为“已批准”(与“请求更改”的方式相同). 或者(对于维护者):如果您认为 pull request 准备好合并,请合并它.
在您自己的机器上检出 pull request 代码的副本以便在本地使用它可能会很有帮助. 您可以通过单击 PR 页面右上角的“使用…打开”按钮,使用 GitHub CLI 来执行此操作.
假设您已设置好您的 development environment ,您现在可以构建代码并对其进行测试.
用于审查的标准回复#
将其中一些存储在 GitHub 的 saved replies 中可能有助于审查:
- 使用问题
You are asking a usage question. The issue tracker is for bugs and new features. I'm going to close this issue, feel free to ask for help via our [help channels](https://numpy.org/gethelp/).
- 欢迎您更新文档
Please feel free to offer a pull request updating the documentation if you feel it could be improved.
- 用于错误的自包含示例
Please provide a [self-contained example code](https://stackoverflow.com/help/mcve), including imports and data (if possible), so that other contributors can just run it and reproduce your issue. Ideally your example code should be minimal.
- 软件版本
To help diagnose your issue, please paste the output of: ``` python -c 'import numpy; print(numpy.version.version)' ``` Thanks.
- 代码块
Readability can be greatly improved if you [format](https://help.github.com/articles/creating-and-highlighting-code-blocks/) your code snippets and complete error messages appropriately. You can edit your issue descriptions and comments at any time to improve readability. This helps maintainers a lot. Thanks!
- 链接到代码
For clarity's sake, you can link to code like [this](https://help.github.com/articles/creating-a-permanent-link-to-a-code-snippet/).
- 更好的描述和标题
Please make the title of the PR more descriptive. The title will become the commit message when this is merged. You should state what issue (or PR) it fixes/resolves in the description using the syntax described [here](https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword).
- 需要回归测试
Please add a [non-regression test](https://en.wikipedia.org/wiki/Non-regression_testing) that would fail at main but pass in this PR.
- 不要更改无关的内容
Please do not change unrelated lines. It makes your contribution harder to review and may introduce merge conflicts to other pull requests.