Рекомендации для рецензентов#

Просмотр открытых pull requests (PRs) помогает продвигать проект вперед. Мы поощряем участие людей вне проекта; это отличный способ ознакомиться с кодом.

Кто может быть рецензентом?#

Рецензии могут поступать от лиц, не входящих в команду NumPy — мы приветствуем вклад от экспертов в предметной области (например, linalg или fft) или сопровождающих других проектов. Вам не обязательно быть сопровождающим NumPy (членом команды NumPy с правом слияния PR) для проведения рецензирования.

Если мы вас еще не знаем, рассмотрите возможность представиться в список рассылки или Slack прежде чем вы начнёте проверять пулл-реквесты.

Руководство по коммуникации#

  • Каждый PR, хороший или плохой, является актом щедрости. Начало с положительного комментария поможет автору почувствовать себя вознаграждённым, и ваши последующие замечания могут быть услышаны более чётко. Вы также можете почувствовать себя хорошо.

  • Начните, если возможно, с крупных проблем, чтобы автор знал, что его поняли. Удерживайтесь от соблазна сразу перейти к построчному разбору или начать с мелких повсеместных проблем.

  • Вы — лицо проекта, и NumPy некоторое время назад решил тип проекта, которым он будет: открытый, эмпатичный, гостеприимный, дружелюбный и терпеливый. Будьте kind участникам.

  • Не позволяйте идеальному быть врагом хорошего, особенно для документации. Если вы делаете много мелких предложений или слишком придираетесь к стилю или грамматике, рассмотрите возможность слияния текущего PR, когда все важные вопросы решены. Затем либо отправьте коммит напрямую (если вы сопровождающий), либо откройте последующий PR самостоятельно.

  • Если вам нужна помощь в написании ответов в обзорах, ознакомьтесь с некоторыми стандартные ответы для проверки.

Контрольный список рецензента#

  • Ясно ли предполагаемое поведение при всех условиях? На что стоит обратить внимание:

    • Что происходит с неожиданными входными данными, такими как пустые массивы или значения nan/inf?

    • Проверяются ли аргументы оси или формы на int или кортежи?

    • Необычные dtypes проверено, поддерживает ли функция их?

  • Следует ли улучшить имена переменных для ясности или согласованности?

  • Следует ли добавлять комментарии или, наоборот, удалять их как бесполезные или избыточные?

  • Соответствует ли документация Рекомендации NumPy? Правильно ли оформлены строки документации?

  • Следует ли код стандартам NumPy Стилистические рекомендации?

  • Если вы сопровождающий, и это не очевидно из описания PR, добавьте краткое объяснение того, что сделала ветка, в сообщение о слиянии и, если закрываете issue, также добавьте "Closes gh-123", где 123 — номер issue.

  • Для изменений кода хотя бы один сопровождающий (т.е. кто-то с правами коммита) должен проверить и одобрить pull request. Если вы первый, кто проверяет PR и одобряет изменения, используйте GitHub одобрить рецензию инструмент, чтобы пометить его как таковой. Если PR простой, например, это явно правильное исправление ошибки, его можно объединить сразу. Если он более сложный или изменяет публичный API, пожалуйста, оставьте его открытым хотя бы на пару дней, чтобы другие сопровождающие получили возможность проверить.

  • Если вы последующий рецензент уже одобренного PR, пожалуйста, используйте тот же метод рецензирования, что и для нового PR (сосредоточьтесь на более крупных проблемах, сопротивляйтесь искушению добавить только несколько придирок). Если у вас есть права на коммит и вы считаете, что дальнейшая рецензия не нужна, объедините PR.

Для сопровождающих#

  • Убедитесь, что все автоматические тесты CI проходят перед слиянием PR, и что сборка документации без каких-либо ошибок.

  • В случае конфликтов слияния попросите автора PR перебазировать на main.

  • Для PR, которые добавляют новые функции или являются сложными, подождите хотя бы день или два перед слиянием. Это даст другим возможность прокомментировать перед тем, как код будет включен. Рассмотрите добавление в примечания к выпуску.

  • При слиянии вкладов коммиттер отвечает за обеспечение соответствия требованиям, изложенным в Руководства по процессу разработки для NumPy. Также проверьте, что новые функции и нарушения обратной совместимости обсуждались на список рассылки numpy-discussion.

  • Сжатие коммитов или очистка сообщений коммитов PR, которые вы считаете слишком запутанными, допустимо. Не забудьте сохранить имя оригинального автора при этом. Убедитесь, что сообщения коммитов следуют правила для NumPy.

  • Когда вы хотите отклонить PR: если это очень очевидно, вы можете просто закрыть его и объяснить почему. Если нет, то хорошей идеей будет сначала объяснить, почему вы считаете, что PR не подходит для включения в NumPy, а затем позволить второму коммиттеру прокомментировать или закрыть.

  • Если автор PR не отвечает на ваши комментарии в течение 6 месяцев, переместите рассматриваемый PR в категорию неактивных с прикрепленным тегом "inactive". В этот момент PR может быть закрыт сопровождающим. Если есть интерес к завершению рассматриваемого PR, это может быть указано в любое время, не дожидаясь 6 месяцев, с помощью комментария.

  • Мейнтейнеры призываются завершать PR, когда необходимы только небольшие изменения перед слиянием (например, исправление стиля кода или грамматических ошибок). Если PR становится неактивным, мейнтейнеры могут вносить более крупные изменения. Помните, что PR — это сотрудничество между контрибьютором и ревьюером/ами, иногда прямой пуш — лучший способ его завершить.

Изменения API#

Как упоминалось, большинство изменений публичного API следует обсуждать заранее и часто с более широкой аудиторией (в списке рассылки или даже через NEP).

Для изменений в публичном C-API учтите, что C-API NumPy обратно совместим, поэтому любое добавление должно быть 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, во вкладке «Изменённые файлы», кнопка «Проверить изменения» (вверху справа).

  • Если вы довольны текущим статусом, отметьте pull request как Approved (так же, как Changes requested). Альтернативно (для сопровождающих): объедините pull request, если считаете, что он готов к слиянию.

Может быть полезно иметь копию кода запроса на вытягивание, проверенную на вашем собственном компьютере, чтобы можно было экспериментировать с ней локально. Вы можете использовать GitHub CLI для этого, нажав на Open with кнопки в правом верхнем углу страницы PR.

Предполагая, что у вас есть среда разработки настроен, теперь вы можете собрать код и протестировать его.

Стандартные ответы для рецензирования#

Может быть полезно хранить некоторые из них в сохраненные ответы для проверки:

Вопрос по использованию
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.