审阅者指南#

审查开放的拉取请求(PR)有助于推动项目前进.我们鼓励项目外的人也参与进来;这是熟悉代码库的一个好方法.

谁可以是审阅者?#

评论可以来自NumPy团队之外——我们欢迎领域专家(例如,`linalg`或`fft`)或其他项目维护者的贡献.您不需要成为NumPy维护者(有权限合并PR的NumPy团队成员)来进行审查.

如果我们还不认识你,建议在开始审查拉取请求之前,在 邮件列表或Slack 上介绍一下自己.

沟通指南#

  • 每个PR,无论好坏,都是一种慷慨的行为.以积极的评论开始将帮助作者感到被奖励,而你随后的评论可能会被更清楚地听到.你也可能会感觉良好.

  • 如果可能,从大的问题开始,这样作者就知道他们已经被理解了.抵制立即逐行进行或从小问题开始的习惯.

  • 你是项目的门面,NumPy 在一段时间前决定 它将成为什么样的项目:开放、富有同情心、欢迎、友好和耐心.对贡献者要 友善.

  • 不要让完美成为好的敌人,尤其是在文档方面.如果你发现自己提出了很多小建议,或者对风格或语法过于挑剔,考虑在所有重要问题得到解决后合并当前的PR.然后,如果你是维护者,可以直接推送一个提交,或者自己打开一个后续的PR.

  • 如果你需要帮助撰写评审回复,可以查看一些 评审的标准回复.

审核者清单#

  • 在所有条件下,预期行为是否清晰?需要注意的一些事项:
    • 对于像空数组或 nan/inf 值这样的意外输入会发生什么?

    • 轴或形状参数是否被测试为 inttuples?

    • 如果一个函数支持这些,是否会测试不寻常的 dtypes?

  • 变量名是否应为了清晰性或一致性而改进?

  • 应该添加评论,还是应该删除那些无用或多余的评论?

  • 文档是否遵循 NumPy 指南?文档字符串是否正确格式化?

  • 代码是否遵循 NumPy 的 风格指南?

  • 如果你是维护者,并且从PR描述中不明显,请在合并消息中添加一个简短的解释,说明分支做了什么,如果关闭一个问题,还要添加”Closes gh-123”,其中123是问题编号.

  • 对于代码更改,至少需要一名维护者(即拥有提交权限的人)审查并批准拉取请求.如果你是第一个审查并批准更改的人,请使用 GitHub 批准审查 工具将其标记为已批准.如果拉取请求很简单,例如它是一个明显正确的错误修复,可以直接合并.如果它更复杂或更改了公共 API,请至少开放几天,以便其他维护者有机会审查.

  • 如果你是一个已经批准的PR的后续评审者,请使用与新PR相同的评审方法(专注于更大的问题,抵制只添加一些吹毛求疵的诱惑).如果你有提交权限并且认为不需要进一步评审,请合并PR.

对于维护者#

  • 在合并PR之前,确保所有自动化的CI测试通过,并且 文档构建 没有任何错误.

  • 如果出现合并冲突,请要求 PR 提交者 在主分支上变基.

  • 对于添加新功能或以某种方式复杂的PR,请在合并之前至少等待一两天.这样,其他人就有机会在代码进入之前发表评论.考虑将其添加到发布说明中.

  • 在合并贡献时,提交者负责确保这些贡献符合 NumPy 的 开发流程指南 中列出的要求.此外,检查新功能和向后兼容性破坏是否已在 numpy-discussion 邮件列表 上讨论过.

  • 压缩提交或清理你认为过于混乱的PR的提交消息是可以的.记得在这样做时保留原作者的名字.确保提交消息遵循 NumPy 的规则.

  • 当你想拒绝一个PR时:如果非常明显,你可以直接关闭它并解释原因.如果不是,那么最好先解释为什么你认为这个PR不适合包含在NumPy中,然后让第二个提交者评论或关闭.

  • 如果 PR 提交者 6 个月内没有回应你的评论,请将该 PR 移至不活跃类别,并附上”inactive”标签.此时,维护者可以关闭该 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

因为在NumPy 1.20中,``buffer_fmt`` 字段被添加到其末尾(所有之前的字段保持ABI兼容).同样地,在 numpy/_core/code_generators/numpy_api.py 中添加到API表的任何函数都必须使用 MinVersion 注解.例如:

'PyDataMem_SetHandler':                 (304, MinVersion("1.22")),

仅头文件功能(例如一个新的宏)通常不需要被保护.

GitHub 工作流程#

在审查拉取请求时,请根据需要在GitHub上使用工作流跟踪功能:

  • 在您完成审查后,如果您想要求提交者进行更改,请将您的审查状态更改为”需要更改”.这可以在 GitHub 的 PR 页面、文件更改选项卡、审查更改(右上角的按钮)中完成.

  • 如果你对当前状态感到满意,请将拉取请求标记为已批准(与请求更改的方式相同).或者(对于维护者):如果你认为拉取请求已准备好合并,请合并它.

在您自己的机器上检出拉取请求代码的副本可能会很有帮助,这样您就可以在本地进行测试.您可以使用 GitHub CLI 通过点击 PR 页面右上角的 Open with 按钮来完成此操作.

假设你已经设置好了你的 开发环境 ,你现在可以构建代码并测试它.

标准回复用于审查#

将其中一些存储在 GitHub 的 已保存的回复 中可能会有所帮助,以便于审阅:

Usage question
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.
Software versions
To help diagnose your issue, please paste the output of:
```
python -c 'import numpy; print(numpy.version.version)'
```
Thanks.
Code blocks
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!
Linking to code
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).
Regression test needed
Please add a [non-regression test](https://en.wikipedia.org/wiki/Non-regression_testing) that would fail at main but pass in this PR.
Don’t change unrelated
Please do not change unrelated lines. It makes your contribution harder to review and may introduce merge conflicts to other pull requests.