Skip to content

Conversation

kavyasrinet
Copy link

This PR solves the issue : #7335

As of now, I have added comments in just the is_parameter function of io.py and made sure this works and renders fine, and added the required rst file.
Once merged, I can add comments to other functions of the file as well in order for them to render.

@kavyasrinet kavyasrinet requested review from cs2be and luotao1 January 12, 2018 02:09
Copy link
Contributor

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

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

Can you paste the generated result like #7445 (comment) to help check the correct format?




isParameter
Copy link
Contributor

Choose a reason for hiding this comment

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

is_parameter

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

var : The input variable.
Returns:
boolean result whether the variable is a Parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

Is line 14 and line 22 lacking periods in the end?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you! I will fix these things. Just wanted to check if it renders the comment at all.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

]


def is_parameter(var):
Copy link
Contributor

Choose a reason for hiding this comment

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

As is_parameter is not in __all__, could it show correctly on the website?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, see screenshot below

@kavyasrinet
Copy link
Author

Here is a screesnhot after adding the rst:
screen shot 2018-01-11 at 7 22 56 pm

@luotao1
Copy link
Contributor

luotao1 commented Jan 12, 2018

From the screenshot, there is no heading isParameter here.

@kavyasrinet
Copy link
Author

Ah right, that was a stale screenshot before I renamed it, here is the updated one:
screen shot 2018-01-11 at 7 50 57 pm

Copy link
Contributor

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

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

LGTM!!

@luotao1 luotao1 merged commit b4d1811 into PaddlePaddle:develop Jan 12, 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.

2 participants