Skip to content

Conversation

@sethah
Copy link
Contributor

@sethah sethah commented May 28, 2018

I noticed that torch.slice does not have a docstring (I assume it is an oversight?), so this patch adds docstrings for torch.slice and tensor.slice.

I built docs with make html and inspected them. If there are other checks or changes that need to be made that I have missed, please let me know.

:attr:`dim` between the indices given by integer inputs :attr:`start` and
:attr:`stop` with the corresponding :attr:`step`, which is an integer.
The returned tensor has the same number of dimensions as the original tensor

This comment was marked as off-topic.

The returned tensor has the same number of dimensions as the original tensor
(:attr:`input`). The size of the :attr:`dim`\ th dimension is given by
``ceil((min(stop, input.size(dim)) - start) / step)``

This comment was marked as off-topic.

@sethah
Copy link
Contributor Author

sethah commented May 29, 2018

I'm not sure the test failures are relevant?

@zou3519
Copy link
Contributor

zou3519 commented May 29, 2018

@pytorchbot retest this please

@colesbury
Copy link
Member

I'm not sure we want to expose torch.slice. All the functionality is present using indexing notation.

I think we might just want to avoid exposing this to Python.

cc @apaszke @gchanan

@apaszke
Copy link
Contributor

apaszke commented May 29, 2018

I think it shouldn't be part of the Python API. It has no benefits over regular indexing.

@sethah
Copy link
Contributor Author

sethah commented May 29, 2018

Ok, thanks for the feedback. I would propose closing this PR and opening a new one that removes the Python bindings for the slice method.

@sethah
Copy link
Contributor Author

sethah commented May 29, 2018

Closing this in favor of #7924.

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.

5 participants