Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

【PaddlePaddle Hackathon 3】16 新增 API paddle.take #5099

Merged
merged 22 commits into from Aug 30, 2022

Conversation

S-HuaBomb
Copy link
Contributor

@S-HuaBomb S-HuaBomb commented Jul 30, 2022

增加 paddle.take 的中文文档。

更新后的 RFC 的链接为:20220714_api_design_for_take.md
该算子的实现 PR 链接为:PaddlePaddle/Paddle#44741
PADDLEPADDLE_PR=44741

@paddle-bot
Copy link

paddle-bot bot commented Jul 30, 2022

感谢你贡献飞桨文档,文档预览构建中,Docs-New 跑完后即可预览,预览链接:http://preview-pr-5099.paddle-docs-preview.paddlepaddle.org.cn/documentation/docs/zh/api/index_cn.html
预览工具的更多说明,请参考:[Beta]飞桨文档预览工具

take
-------------------------------

.. py:function:: paddle.take(input, index, name)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

take(input, index, name=None)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

返回
:::::::::

- `Tensor` - 返回一个新的 Tensor,其中包含给定索引处的输入元素。结果与 `index` 的形状相同。
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Tensor和index为什么要使用斜体?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已经改回正体,
Done

@@ -2412,3 +2412,12 @@ erfinv(x, name=None)
对输入 x 进行逆误差函数计算

请参考 :ref:`cn_api_paddle_tensor_erfinv`

take(input, index, name=None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

作为 Tensor 的方法应该不需要第一个参数~

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

docs/api/paddle/take_cn.rst Outdated Show resolved Hide resolved
@@ -1,18 +1,17 @@
.. _cn_api_paddle_vision_models_alexnet:
.. _cn_api_paddle_vision_models_AlexNet:
Copy link
Member

@SigureMo SigureMo Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

注意不要把没有修改的文件提交了哦,这是 PaddlePaddle/Paddle#44671 提到的问题,暂时还没办法解决,也确实容易误操作,不过暂时先自己注意一下吧~如果可以的话尽可能恢复下~(大小写不敏感文件系统可能不太好处理,如果不能直接恢复的话,可以试试比如 revert 的命令来恢复,实在不行的话只能重新提 PR 了)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

感谢建议。这个问题的确很令人困扰,可以按照你们的方案避免同名。
Done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx,Done

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个问题现在有团队在解决了,预期Q3完成~

take(input, index, name=None)
:::::::::

返回:一个新的 Tensor,其中包含给定索引处的输入元素。结果与 `index` 的形状相同
Copy link
Member

@SigureMo SigureMo Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里还有一个用 ` 包裹的,也许我们应该用

:attr:`arg_name`

这样?目前 Paddle 文档中引用某一个参数经常会使用这个,比如 argmax 源码和渲染分别为

image

image

这里看起来虽然与普通行内代码块一样,但应该仅仅是 Paddle 前端没有为两者样式做区分,比如 sphinx-rtd-theme 两者区别就是比较明显的

image

@BrilliantYuKaimin 有什么建议嘛?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在 rst 中用一个 ` 包裹的表示斜体,用两个 ` 包裹的表示代码块。既然 :attr:`arg_name` 和 ``arg_name`` 确实可以渲染出不同的效果,那么当然应该使用 :attr:`arg_name`。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx,Done

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

「仅仅是 Paddle 前端没有为两者样式做区分」
给前端加一个需求吧

@SigureMo
Copy link
Member

SigureMo commented Aug 7, 2022

image

看样子还有些代码风格检查没有过,需要本地安装 pre-commit(这个 repo 里也需要初始化下),之后相应的文件随便加点什么后 commit 以触发 pre-commit 的自动修复(最好是些没什么影响的,比如在某一行后加些空格,这些之后会在触发 pre-commit 后自动删除,如果没有自动删的话手动记得删一下 😂)

@S-HuaBomb
Copy link
Contributor Author

image

看样子还有些代码风格检查没有过,需要本地安装 pre-commit(这个 repo 里也需要初始化下),之后相应的文件随便加点什么后 commit 以触发 pre-commit 的自动修复(最好是些没什么影响的,比如在某一行后加些空格,这些之后会在触发 pre-commit 后自动删除,如果没有自动删的话手动记得删一下 😂)

很奇怪,我修改 take_cn.rst 之后 commit 却没有触发 pre-commit,环境中已经安装了。不过 take_cn.rst 的问题应该是最后空了两行,我手动删了一行。
THX,done

@Ligoml
Copy link
Collaborator

Ligoml commented Aug 8, 2022

image 看样子还有些代码风格检查没有过,需要本地安装 pre-commit(这个 repo 里也需要初始化下),之后相应的文件随便加点什么后 commit 以触发 pre-commit 的自动修复(最好是些没什么影响的,比如在某一行后加些空格,这些之后会在触发 pre-commit 后自动删除,如果没有自动删的话手动记得删一下 😂)

很奇怪,我修改 take_cn.rst 之后 commit 却没有触发 pre-commit,环境中已经安装了。不过 take_cn.rst 的问题应该是最后空了两行,我手动删了一行。 THX,done

因为这个格式检查的pr上周五合入的, @SigureMo 贡献的pr。目前看来CI还是会卡住,再检查一下吧~

@S-HuaBomb
Copy link
Contributor Author

在新的仓库中使用 pre-commit 需要先运行 pre-commit install

我用 pre-cpmmit 会有这个问题:
image
它检查了之前提到的由于大小写出问题的文件,因为它们一直被标为修改文件在暂存区,restore 也没有用。我删除了空了两行的地方,这次应该好了。
Thx,done

@SigureMo
Copy link
Member

SigureMo commented Aug 8, 2022

在新的仓库中使用 pre-commit 需要先运行 pre-commit install。

嗯嗯,首先全局用 pip install pre-commit 是全局安装这个工具(一次即可)

之后每个 repo 都需要 pre-commit install 将 git hooks 注册到具体的这一个 repo

我用 pre-commit 会有这个问题:

嗯对的,我本地也用不了 pre-commit,现在都是在 codespace 上修改文档开发的(那个 issue 里我也有说),就很麻烦……

@@ -136,6 +136,7 @@ tensor 数学操作
" :ref:`paddle.acosh <cn_api_fluid_layers_acosh>` ", "反双曲余弦函数"
" :ref:`paddle.asinh <cn_api_fluid_layers_asinh>` ", "反双曲正弦函数"
" :ref:`paddle.atanh <cn_api_fluid_layers_atanh>` ", "反双曲正切函数"
" :ref:`paddle.take <cn_api_tensor_take>` ", "输出给定索引处的输入元素,结果与 index 的形状相同"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

还有问题,这个是说在句尾多了一个空格

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx,done

Comment on lines 16 to 19
- **mode** (str,可选) - 索引越界处理,可选 ``'raise'``,``'wrap'``,``'clip'``,默认为 ``'raise'``。
- ``raise``:直接抛出错误;
- ``wrap``:通过取余数来约束超出范围的索引;
- ``clip``:将超出范围的索引剪裁到允许的最小(大)范围;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

嵌套列表需要上下各空一行。可参考 reStructuredText Primer(Lists and Quote-like blocks)

英文这里保持一致格式吧,也用列表的形式,不过可能也需要注意添加空行

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx,done

Copy link
Collaborator

@Ligoml Ligoml left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good job!LGTM

@luotao1 luotao1 merged commit e249d63 into PaddlePaddle:develop Aug 30, 2022
HermitSun pushed a commit to HermitSun/paddle-docs that referenced this pull request Aug 31, 2022
* add docs for paddle.take

* fix docs

* fix conflict

* fix conflict

* Update docs/api/paddle/take_cn.rst

Co-authored-by: Nyakku Shigure <sigure.qaq@gmail.com>

* fix docs

* revert commit 498c668

* fix revert 498c668

* Revert "fix conflict"

This reverts commit 6d7633e.

* Revert "Revert "fix conflict""

This reverts commit e2d5299.

fix e2d5299#

* fix docs

* fix take_cn by pre-commit

* fix take_cn by pre-commit

* fix take_cn by pre-commit

* fix take_cn by pre-commit

* fix take_cn.rst by pre-commit

* fix Overview_cn.rst

* fix doc, add param 'mode'

* fix 嵌套列表

Co-authored-by: Nyakku Shigure <sigure.qaq@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants