Skip to content

Conversation

@f0k
Copy link
Contributor

@f0k f0k commented Oct 18, 2018

As discussed in #1570, this adds a warning to the docstring of tensor.resize_() to prevent people from naively using it as an in-place view or reshape.

For your convenience, the updated docstring renders as follows:
torch_resize_docstring

Fixes #1570.

Copy link
Collaborator

@ssnl ssnl left a comment

Choose a reason for hiding this comment

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

:) Thanks! set_ is also very dangerous, and also deserve a warning. Let me know if you want to do that change too in this PR. If not, I'll merge this as-is.

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 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@f0k
Copy link
Contributor Author

f0k commented Oct 19, 2018

Thanks for merging!

set_ is also very dangerous, and also deserve a warning.

I think the arguments and description of set_ are intimidating enough to defer accidental misuse. What would you warn against? resize_(*sizes) looked pretty innocent, like an in-place reshape(*shape) with an inconsistent name.

@ssnl
Copy link
Collaborator

ssnl commented Oct 23, 2018

@f0k Your reasoning makes sense :). Thanks again for the PR.

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.

Make resize_ and resize_as_ internal methods

4 participants