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

Simplize API Reference Documentation #11308

Merged
merged 5 commits into from
Jun 11, 2018

Conversation

reyoung
Copy link
Collaborator

@reyoung reyoung commented Jun 8, 2018

default

default

default

default

Polish fill_constant_batch_size_like, load, max_sequence_len, resize_bilinear2d:

  • Add missing example
  • Move load from tensor to io
  • Use C++ comments instead of copying them twice.

@reyoung reyoung requested a review from JiayiFeng June 8, 2018 07:51
"with the specified value");
AddAttr<std::vector<int>>("shape", "(vector<int>) The shape of the output");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is stupid to write type again. The type has been registered by template arguments.

@reyoung reyoung requested a review from shanyi15 June 8, 2018 07:55
@skylarch
Copy link
Collaborator

skylarch commented Jun 8, 2018

第一个API的indentation,从“It also sets..."开始错位。

@reyoung
Copy link
Collaborator Author

reyoung commented Jun 8, 2018

@skylarch Done.

@JiayiFeng
Copy link
Collaborator

max_seuqence_len的文档多了一项“返回类型”

@reyoung
Copy link
Collaborator Author

reyoung commented Jun 8, 2018

@JiayiFeng Done

@reyoung
Copy link
Collaborator Author

reyoung commented Jun 8, 2018

Also polish the math equations in crf layer

default

Copy link
Collaborator

@JiayiFeng JiayiFeng left a comment

Choose a reason for hiding this comment

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

LGTM. But I think we'd better let @shanyi15 take a look at this before merging it.

@reyoung reyoung mentioned this pull request Jun 8, 2018
comment += escape_inline_math(line)
comment += " "
elif len(comment) != 0:
comment += "\n \n "
Copy link
Contributor

Choose a reason for hiding this comment

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

Why need to add spaces at then end?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It correctly indent the comment to 4 spaces. It needed for google style comments.

It is tricky now. It should use regex to detect indent length.

elif len(comment) != 0:
comment += "\n \n "

args = {"comment": trim_ending_dot(comment)}
for each_input in op_proto.inputs:
input_name = _convert_(each_input.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

_convert_ seems will output wired string when input is something like ROIs, outputs ro_is. Just put out this problem.

Or maybe we can just check that name is valid?

@shanyi15
Copy link
Collaborator

@skylarch , please recheck the format

@reyoung reyoung merged commit 9328c3c into PaddlePaddle:develop Jun 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants