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

[Typing][B-50,B-51] Add type annotations for python/paddle/static/{io,input.py} #67047

Merged
merged 6 commits into from
Aug 16, 2024

Conversation

ooooo-create
Copy link
Contributor

PR Category

User Experience

PR Types

Improvements

Description

类型标注:
- python/paddle/static/io.py
- python/paddle/static/input.py

Related links

@SigureMo @megemini

Copy link

paddle-bot bot commented Aug 5, 2024

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

@paddle-bot paddle-bot bot added the contributor External developers label Aug 5, 2024
@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Aug 6, 2024
def setitem(
x: Tensor,
index: int | slice | Sequence[int] | Tensor | None,
value: Tensor | npt.NDArray[Any] | bool | complex,
Copy link
Contributor

Choose a reason for hiding this comment

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

_typing 里面有 Numberic ,或者直接用 TensorLike ?

inference_program, feed_target_names, feed_holder_name='feed'
):
inference_program: Program,
feed_target_names: list[str],
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
feed_target_names: list[str],
feed_target_names: Sequence[str],

输入的地方如果没有特殊要求(如 list 的 append 方法,或者用 isinstance 进行判断等),最好用 Sequence 吧 ~

后面有几个也一样 ~

program: Program,
feed_vars: Tensor | list[Tensor],
fetch_vars: Tensor | list[Tensor],
**kwargs: Any,
Copy link
Contributor

Choose a reason for hiding this comment

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

可以用 TypedDict

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已添加~

def serialize_program(
feed_vars: Tensor | list[Tensor],
fetch_vars: Tensor | list[Tensor],
**kwargs: Any,
Copy link
Contributor

Choose a reason for hiding this comment

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

TypedDict ?后面的几个是不是也可以?

def load_inference_model(path_prefix, executor, **kwargs):
def load_inference_model(
path_prefix: str | None, executor: Executor, **kwargs: Any
) -> list[Program, list[str], list[Tensor]]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
) -> list[Program, list[str], list[Tensor]]:
) -> list[Program | list[str] | list[Tensor]]:

list 只能有一个参数 ~ 其实,代码用 tuple 返回更好 ... ...

python/paddle/static/io.py Outdated Show resolved Hide resolved
python/paddle/static/io.py Outdated Show resolved Hide resolved
Copy link

paddle-ci-bot bot commented Aug 13, 2024

Sorry to inform you that e7d0910's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@megemini
Copy link
Contributor

@ooooo-create 别忘了这里还有个任务 🫣

@ooooo-create
Copy link
Contributor Author

@ooooo-create 别忘了这里还有个任务 🫣

抱歉抱歉,收到~

@PaddlePaddle PaddlePaddle locked and limited conversation to collaborators Aug 15, 2024
@PaddlePaddle PaddlePaddle unlocked this conversation Aug 15, 2024
@@ -19,6 +20,7 @@
import pickle
import sys
import warnings
from typing import TYPE_CHECKING, Any, TypedDict, Unpack, overload
Copy link
Contributor

Choose a reason for hiding this comment

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

Unpack 在 typing_extensions 里面 ~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done

path_prefix: str | None,
executor: Executor,
**kwargs: Unpack[_LoadInferenceModelKwargs],
) -> list[Program | list[str] | list[Tensor]]:
Copy link
Contributor

Choose a reason for hiding this comment

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

原代码写的有点奇怪,一般没有把返回的 tuple 转为 list 的,没什么意义 ... ...

这也导致示例代码中无法正确判断返回值的类型。

ignore 一下示例吧

            >>> results = exe.run(inference_program,  # type: ignore[arg-type]
            ...               feed={feed_target_names[0]: tensor_img},  # type: ignore[index,dict-item]
            ...               fetch_list=fetch_targets)  # type: ignore[arg-type]

Copy link
Contributor

@megemini megemini left a comment

Choose a reason for hiding this comment

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

LGTM ~ 🤟

Copy link
Member

@SigureMo SigureMo left a comment

Choose a reason for hiding this comment

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

LGTMeow 🐾

@luotao1 luotao1 merged commit 09cedfa into PaddlePaddle:develop Aug 16, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers HappyOpenSource 快乐开源活动issue与PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants