审稿人指南#
审查开放的拉取请求 (PR) 有助于推动项目向前发展。我们鼓励项目之外的人也参与进来;这是熟悉代码库的好方法。
谁可以成为审稿人?#
评论可以来自 NumPy 团队外部 - 我们欢迎领域专家(例如linalg或fft)或其他项目维护者的贡献。您无需成为 NumPy 维护者(有权合并 PR 的 NumPy 团队成员)即可进行审查。
如果我们还不认识您,请考虑在开始审查拉取请求之前在邮件列表或 Slack中介绍您自己。
沟通指南#
每一次公关,无论好坏,都是一种慷慨之举。以积极的评论开头会让作者感到有所收获,并且你后续的评论可能会被更清楚地听到。你可能也会感觉良好。
如果可能的话,从大问题开始,这样作者就知道它们已经被理解了。抵制住立即逐行检查或以普遍存在的小问题开始的诱惑。
不要让完美成为优秀的敌人,特别是对于文档而言。如果您发现自己提出了许多小建议,或者对风格或语法过于挑剔,请考虑在解决所有重要问题后合并当前的 PR。然后,要么直接推送提交(如果您是维护者),要么自己打开后续 PR。
如果您需要帮助撰写评论回复,请查看一些 标准评论回复。
审稿人清单#
- 在所有条件下预期行为是否明确?一些值得注意的事情:
空数组或 nan/inf 值等意外输入会发生什么情况?
axis 或 shape 参数是否测试为int或tuples?
如果函数支持异常数据类型,是否会测试这些异常数据类型?
是否应该改进变量名称以使其清晰或一致?
是否应该添加评论,或者更确切地说,将其视为无益或无关的评论而删除?
该文档是否遵循NumPy 指南?文档字符串的格式是否正确?
代码是否遵循 NumPy 的风格指南?
如果您是维护者,并且从 PR 描述中看不出来,请添加一个简短的说明,说明分支对合并消息所做的操作,如果关闭问题,还请添加“关闭 gh-123”,其中 123 是问题编号。
对于代码更改,至少一名维护者(即具有提交权限的人)应审查并批准拉取请求。如果您是第一个审核 PR 并批准更改的人,请使用 GitHub批准审核工具对其进行标记。如果 PR 很简单,例如它是一个明显正确的错误修复,则可以立即合并。如果它更复杂或更改了公共 API,请将其开放至少几天,以便其他维护人员有机会进行审查。
如果您是已批准 PR 的后续审核者,请使用与新 PR 相同的审核方法(关注更大的问题,抵制仅添加一些挑剔的诱惑)。如果您有提交权并且认为不需要更多审查,请合并 PR。
对于维护者#
确保在合并 PR 之前所有自动化 CI 测试都通过,并且 文档的构建没有任何错误。
如果发生合并冲突,请要求 PR 提交者在 main 上重新建立基础。
对于添加新功能或在某种程度上复杂的 PR,请至少等待一两天再合并。这样,其他人就有机会在代码输入之前发表评论。考虑将其添加到发行说明中。
合并贡献时,提交者负责确保这些贡献满足NumPy开发流程指南中概述的要求。另外,请检查numpy-discussion 邮件列表上是否讨论了新功能和向后兼容性中断。
压缩提交或清理您认为太混乱的 PR 的提交消息是可以的。执行此操作时请记住保留原作者的姓名。确保提交消息遵循NumPy 的规则。
当你想拒绝 PR 时:如果它非常明显,你可以关闭它并解释原因。如果不是,那么最好首先解释为什么您认为 PR 不适合包含在 NumPy 中,然后让第二个提交者发表评论或关闭。
如果 PR 提交者在 6 个月内没有回复您的评论,请将相关 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 兼容)。同样,添加到 API 表中的任何函数
numpy/core/code_generators/numpy_api.py
都必须使用该MinVersion
注释。例如:
'PyDataMem_SetHandler': (304, MinVersion("1.22")),
仅标头功能(例如新宏)通常不需要保护。
GitHub 工作流程#
在审查拉取请求时,请酌情使用 GitHub 上的工作流程跟踪功能:
审核完成后,如果您想要求提交者进行更改,请将您的审核状态更改为“已请求更改”。这可以在 GitHub、PR 页面、文件更改选项卡、查看更改(右上角的按钮)上完成。
如果您对当前状态感到满意,请将拉取请求标记为已批准(与请求更改的方式相同)。或者(对于维护者):如果您认为已准备好合并拉取请求,则合并拉取请求。
在您自己的计算机上签出拉取请求代码的副本可能会有所帮助,以便您可以在本地使用它。您可以使用GitHub CLI通过单击PR 页面右上角的按钮来执行此操作。Open with
假设您已经设置了开发环境 ,现在可以构建代码并对其进行测试。
审核标准回复#
将其中一些内容存储在 GitHub保存的回复中以供审阅可能会有所帮助:
- 使用问题
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](/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.