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][C-25/26, C-[40-44], C-83][BUAA] Add type annotations for 7 files in python/paddle/distributed/{communication/fleet} and 1 file in python/paddle/incubate #67203

Closed
wants to merge 5 commits into from

Conversation

Whsjrczr
Copy link
Contributor

@Whsjrczr Whsjrczr commented Aug 8, 2024

PR Category

User Experience

PR Types

Others

Description

完成了Typehint标注:
modified: python/paddle/distributed/communication/stream/all_reduce.py
modified: python/paddle/distributed/communication/stream/all_to_all.py
modified: python/paddle/distributed/fleet/base/distributed_strategy.py
modified: python/paddle/distributed/fleet/base/role_maker.py
modified: python/paddle/distributed/fleet/base/topology.py
modified: python/paddle/distributed/fleet/base/util_factory.py
modified: python/paddle/distributed/fleet/data_generator/data_generator.py
modified: python/paddle/incubate/nn/layer/fused_ec_moe.py(冲突,被修改,已修复)

Copy link

paddle-bot bot commented Aug 8, 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.

@luotao1 luotao1 added contributor External developers HappyOpenSource Pro 进阶版快乐开源活动,更具挑战性的任务 labels Aug 8, 2024
@Whsjrczr
Copy link
Contributor Author

请求approve!

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.

问题比较多,先总体修改一遍再 review 吧 ~

另外,这里面文件中需要标注的接口:

  • python/paddle/distributed/fleet/base/distributed_strategy.py : DistributedStrategy
  • python/paddle/distributed/fleet/base/role_maker.py : PaddleCloudRoleMaker, Role, UserDefinedRoleMaker
  • python/paddle/distributed/fleet/base/topology.py : CommunicateTopology, HybridCommunicateGroup, ParallelMode
  • python/paddle/distributed/fleet/base/util_factory.py : UtilBase
  • python/paddle/distributed/fleet/data_generator/data_generator.py : MultiSlotDataGenerator, MultiSlotStringDataGenerator

如果是类,则标注类的公开接口、实例参数以及必要的魔法方法(如 init 等)

如果无法确定哪些接口需要标注,可以查看模块下面 __init__.py__all__ 的引用关系 ~

还有,PR 需要根据模板样式提交,不然无法及时 review

Copy link
Contributor

Choose a reason for hiding this comment

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

#67112 已经 approve ,这里删掉

Copy link
Contributor

Choose a reason for hiding this comment

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

#67112 已经 approve ,这里删掉

Copy link
Contributor

Choose a reason for hiding this comment

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

几个问题

  • 这个文件只有 DistributedStrategy 需要标注,其他的函数不需要标注
  • 不要都使用 object 或者 Any ,分析一下输入输出类型
  • 泛型需要参数化,不要只使用 dict, list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

收到 已提交新PR

megemini

This comment was marked as duplicate.

@Whsjrczr Whsjrczr closed this Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers HappyOpenSource Pro 进阶版快乐开源活动,更具挑战性的任务
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants