Skip to content

Conversation

@olliethomas
Copy link
Contributor

A small change that adds a docstring that can be found with
getattr(nn.Module, nn.Module.forward.__name__, None).__doc__

Fixes #43057

A small change that adds a docstring that can be found with `getattr(nn.Module, nn.Module.forward.__name__, None).__doc__`

Closes pytorch#43057
# forward as a value, rather than a function. See also
# https://github.com/python/mypy/issues/8795
def _forward_unimplemented(self, *input: Any) -> None:
r"""Forward defines the computation performed at every call.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of making this a docstring, I would have expected the string next to forward to be set as the docstring on the forward object and for this to be unchanged. Don't you want the other string as the docstring?

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 for the advice. I wasn't sure how best to do this as I was reluctant to move the docstring for forward away from the definition of forward.

I've made that change.

@mruberry mruberry removed the request for review from apaszke August 14, 2020 17:17
@mruberry mruberry self-requested a review August 14, 2020 18:36
Copy link
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

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

Thanks @olliethomas!

@mruberry mruberry requested a review from ezyang August 14, 2020 18:39
@mruberry
Copy link
Collaborator

@ezyang Looks like you last moved this doc string, want to sanity check this change?

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ezyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@ezyang merged this pull request in e39b43f.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nn.Module.forward has no docstring

5 participants