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

[Add] Paddle 代码 CI 中引入 xdoctest 检查 #55295

Merged
merged 18 commits into from
Jul 18, 2023

Conversation

megemini
Copy link
Contributor

@megemini megemini commented Jul 10, 2023

PR types

New features

PR changes

Others

Description

中国软件开源创新大赛:飞桨框架任务挑战赛
赛题五:将 xdoctest 引入到飞桨框架工作流中

RFC [Add]将 xdoctest 引入到飞桨框架工作流中v1
ISSUE 赛题五:将 xdoctest 引入到飞桨框架工作流中 Tracking Issue

第一阶段
任务:Paddle 代码 CI 中引入 xdoctest 检查

  • sampcd_processor_readme.md : Xdoctester 的详细设计文档
  • sampcd_processor_utils.py : 代码检查的相关工具
  • sampcd_processor_xdoctest.py : Xdoctester 的相关实现
  • sampcd_processor.py : 原代码检查工具
  • test_sampcd_processor.py : 原代码检查工具单元测试
  • test_sampcd_processor_xdoctest.py : Xdoctester 单元测试
  • requirements.txt : 添加了 xdoctest
  • beta.py : 修改了其中的示例代码样式,用于检查新工具 Paddle CI 的正确性。

建议先 review sampcd_processor_readme.md : Xdoctester 的详细设计文档,看看还有哪些需要修改的设计~

另外,xdoctestrequirement 看看放在 python 目录下是否合适?

@SigureMo @luotao1 @jzhang533 @sunzhongkai588

请评审,谢谢!:)

@paddle-bot
Copy link

paddle-bot bot commented Jul 10, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

>>> beta = paddle.distribution.Beta(alpha=0.5, beta=0.5)
>>> print(beta.mean)
Tensor(shape=[1], dtype=float32, place=Place(cpu), stop_gradient=True,
[0.50000000])
Copy link
Member

Choose a reason for hiding this comment

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

image

这里是特意用的错误例子还是?0D Tensor 应该就是 shape=[] 的,2.5 应该就已经支持 0D Tensor 了,你那里用的是 2.4 嘛?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

忘了更新 Paddle 了 ...........................................

@@ -7,3 +7,4 @@ decorator
astor
paddle_bfloat==0.1.7
opt_einsum==3.3.0
xdoctest
Copy link
Member

Choose a reason for hiding this comment

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

xdoctest 的 requirement 看看放在 python 目录下是否合适?

不合适,requirements.txt 会在 setup.py 里用到,作为 paddle 的「依赖项」,用户在安装时候会自动安装

相比于「依赖项」,xdoctest 更类似于「开发依赖项」,不过我们目前是没有一个统一的地方来添加这个的(大多数社区规范是写一个 requirements-dev.txt,但我们没有),我们只有 python/paddle/unittest_py/requirements.txt(即「测试依赖」),但放在里面也是明显不合适的

目前可以添加到流水线中,或许以后可以直接放在 Docker 里

Copy link
Member

Choose a reason for hiding this comment

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

刚和 @luotao1 讨论了一下,这个还是放在 python/paddle/unittest_py/requirements.txt 吧,PR-CI-Static-Check 是会自动安装这个的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK~ 只是 Paddle build 的时候可能也会调用这个代码检查,感觉放哪儿都不太合适~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

刚和 @luotao1 讨论了一下,这个还是放在 python/paddle/unittest_py/requirements.txt 吧,PR-CI-Static-Check 是会自动安装这个的

Paddle/python/unittest_py/requirements.txt 吧?

Copy link
Member

@SigureMo SigureMo Jul 11, 2023

Choose a reason for hiding this comment

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

@luotao1 luotao1 self-assigned this Jul 11, 2023
@SigureMo
Copy link
Member

辛苦,我看这个 PR 最近还在修改一些文档进行测试,是否已经准备好 review 了呢?准备好的时候可以 @ 一下我

另外我们也测试一下这边和 docs 那边能否正常 copy 这边修改过的源码吧,这边在 commit 里加上 test=docs_preview 就可以在 PR-CI-Paddle-Doc-Preview 流水线看到英文文档了

中文文档那边需要新建一个 PR,然后在 PR 描述里添加 PADDLEPADDLE_PR=55295 即可

@megemini
Copy link
Contributor Author

一直在等 review ......... 一直没消息,所以这几天就改了几个 codeblock,暂时也没发现什么问题~

中文文档那边需要新建一个 PR

这个是指?

@SigureMo
Copy link
Member

这个是指?

docs repo

@megemini
Copy link
Contributor Author

这个是指?

docs repo

关于什么的 PR?

@SigureMo
Copy link
Member

SigureMo commented Jul 16, 2023

docs 端才能触发中文文档 preview 呀,随便创建即可

@megemini
Copy link
Contributor Author

docs 端才能触发中文文档 preview 呀,随便创建即可

@SigureMo 建好了~

PaddlePaddle/docs#6031


## Paddle docs 需要注意

目前 Paddle docs 对于 `>>> ` 代码的处理是,strip 掉此提示符,然后交给原有代码检查工具进行检测。这种方法在大部分情况下没什么问题,但是,如果代码中有 `requires` 项,则可能检查失败。所以,后续需要修改 Paddle docs 的检查逻辑,建议对于 `>>> ` 直接跳过,与当前 Paddle 的 `sampcd_processor.py` 一致。最后收尾的时候,移除掉 Paddle docs 的代码检查。
Copy link
Member

Choose a reason for hiding this comment

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

最后收尾的时候,移除掉 Paddle docs 的代码检查。

COPY-FROM 任务结束的时候应该就可以移除掉了

Copy link
Member

Choose a reason for hiding this comment

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

整体上看起来没有问题,但是是否能补充一些原来的文档链接呢?比如 directive 原文档是如何描述的呢?

Copy link
Member

Choose a reason for hiding this comment

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

整体上看起来没有问题,但是是否能补充一些原来的文档链接呢?比如 directive 原文档是如何描述的呢?

@@ -0,0 +1,269 @@
# Copyright (c) 2019 PaddlePaddle Authors. All Rights Reserved.
Copy link
Member

Choose a reason for hiding this comment

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

2023


self.directive_pattern = re.compile(
r"""
(?<=(\#\s{1})) # positive lookbehind, directive begins
Copy link
Member

Choose a reason for hiding this comment

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

这里前面加这么多空格是有必要的吗?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants