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

fix paddle.vision.datasets.* en docs #43649

Merged
merged 9 commits into from
Jun 23, 2022

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Jun 18, 2022

PR types

Others

PR changes

Docs

Describe

PaddlePaddle/docs#4944 的英文文档同步修复 PR

@paddle-bot-old paddle-bot-old bot added contributor External developers status: proposed labels Jun 18, 2022
@paddle-bot-old
Copy link

你的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.

@SigureMo SigureMo marked this pull request as draft June 18, 2022 18:35
@SigureMo
Copy link
Member Author

SigureMo commented Jun 18, 2022

关于代码示例,原来的几个示例风格各异,有的过于简单,不足以表现 API 的特性,有的加上了网络(SimpleNet),感觉有点“强行”,一般没有将一张图片传入模型的,结合模型应该是 DataLoader 那部分进行展示。所以统一了整体的风格,重写了下,基本都分为两部分,前半部分展示默认参数及基本使用方法及属性,后半部分展示非默认参数的定义方式。

如有不合适的地方我再调整。

@SigureMo SigureMo changed the title [WIP, Don't review] fix paddle.vision.datasets.* en docs fix paddle.vision.datasets.* en docs Jun 20, 2022
@SigureMo SigureMo marked this pull request as ready for review June 20, 2022 07:48
python/paddle/vision/datasets/folder.py Show resolved Hide resolved

def make_fake_file(img_path: str):
if img_path.endswith((".jpg", ".png", ".jpeg")):
fake_img = np.random.randint(0, 256, (32, 32, 3), dtype=np.uint8)
Copy link
Contributor

Choose a reason for hiding this comment

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

这里改成paddle.randint?

Copy link
Member Author

Choose a reason for hiding this comment

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

唔,这个的话我也稍微考虑了下,当时写的时候在想要不要使用 Paddle API,但是个人感觉因为这个本身就是 NumPy 数据,并不会直接与 Paddle 交互,就算用 Paddle API 也需要再转成 NumPy 才能作为图片写入硬盘,感觉是否有点多此一举。这个在一般用例中感觉大家都会直接用 NumPy API,所以也许直接用 NumPy API 会好一些?

目前对于一些情况我觉得是毫无疑问可以直接转成 Paddle API 的,比如像先用 NumPy 创建一个数据,然后马上就 to_tensor 这种情况,因为后续这个数据直接就与 Paddle 进行交互了,所以毫无疑问,用 NumPy API 创建一遍再转换是完全没必要的。

但是眼前这个情况感觉是反过来的,所以不清楚这种情况是否有必要用 Paddle API 呢?

Copy link
Contributor

Choose a reason for hiding this comment

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

哦哦我没细看代码,如果是api处理的输入就是numpy格式,那使用numpy创建输入是没有问题的
示例代码原则还是以展示API用法,并且代码尽量简洁优雅为主的~

Copy link
Contributor

@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.

LGTM for docs

@Ligoml
Copy link
Contributor

Ligoml commented Jun 23, 2022

关于代码示例,原来的几个示例风格各异,有的过于简单,不足以表现 API 的特性,有的加上了网络(SimpleNet),感觉有点“强行”,一般没有将一张图片传入模型的,结合模型应该是 DataLoader 那部分进行展示。所以统一了整体的风格,重写了下,基本都分为两部分,前半部分展示默认参数及基本使用方法及属性,后半部分展示非默认参数的定义方式。

如有不合适的地方我再调整。

这个请API负责人 @heavengate 确认一下

Copy link
Contributor

@heavengate heavengate left a comment

Choose a reason for hiding this comment

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

LGTM

@Ligoml Ligoml merged commit 1fca8f3 into PaddlePaddle:develop Jun 23, 2022
@SigureMo SigureMo deleted the fix-vision-datasets-en-docs branch June 23, 2022 08:03
sneaxiy pushed a commit to sneaxiy/Paddle that referenced this pull request Jun 27, 2022
* rewrite all code examples, test=document_fix

* refine arguments, test=document_fix

* fix desc format error, test=document_fix

* capitalize the first letter, test=document_fix

* refine api desc, test=document_fix

* fix wrong COPY-FROM label in Model docs, test=document_fix

* refine returns, test=document_fix

* refine returns, test=document_fix

* add a blank line in code block, test=document_fix
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.

3 participants