Skip to content

C++ API parity: at::Tensor::version #26217

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.

Sign up for GitHub

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

Jump to bottom
Closed
pbelevich wants to merge 23 commits into from
Closed

C++ API parity: at::Tensor::version #26217

pbelevich wants to merge 23 commits into from

Conversation

@pbelevich
Copy link
Contributor

@pbelevich pbelevich commented Sep 13, 2019 •
edited
Loading

Stack from ghstack:

Differential Revision: D17427577

@pytorchbot pytorchbot added module: autograd Related to torch.autograd, and the autograd engine in general module: cpp Related to C++ API module: internals Related to internal abstractions in c10 and ATen module: operators labels Sep 13, 2019
This was referenced Sep 13, 2019
Closed
Closed
Closed
pbelevich added a commit that referenced this pull request Sep 13, 2019
@pbelevich
C++ API parity: at::Tensor::version
20f73af 
ghstack-source-id: dd76920
Pull Request resolved: #26217
pbelevich added a commit that referenced this pull request Sep 15, 2019
@pbelevich
C++ API parity: at::Tensor::version
883dd31 
ghstack-source-id: a08b110
Pull Request resolved: #26217
This was referenced Sep 15, 2019
Closed
Closed
@pytorchbot pytorchbot added the module: pybind Related to our Python bindings / interactions with other Python libraries label Sep 16, 2019
@pbelevich pbelevich mentioned this pull request Sep 17, 2019
Closed
@yf225
Copy link
Contributor

yf225 commented Sep 17, 2019

@pbelevich Sorry I just realized we can actually name the method _version instead of version to better match the Python API, which should also fix the CI test error. The reason I didn't suggest the _version name originally was because having a C++ identifier beginning with an underscore is usually not encouraged, but since we already have a lot of functions that start with an underscore (e.g. func: _copy_from) in native_functions.yaml, it might be acceptable to do so here.

@pbelevich
Copy link
Contributor Author

pbelevich commented Sep 17, 2019

@yf225 def check_methods_do_not_start_with_underscore prevents me from doing this

@yf225
Copy link
Contributor

yf225 commented Sep 17, 2019

@pbelevich for check_methods_do_not_start_with_underscore, we can make an exception for this case and add _version to the whitelist

yf225
yf225 approved these changes Sep 18, 2019
View reviewed changes
Copy link
Contributor

@yf225 yf225 left a comment

Choose a reason for hiding this comment

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

Looks awesome, thanks a lot @pbelevich ! I re-ran the failed CI jobs so that we can get the full set of signals.

@pbelevich
Update on "C++ API parity: at::Tensor::version"
f074354 
Differential Revision: [D17427577](https://our.internmc.facebook.com/intern/diff/D17427577)

[ghstack-poisoned]
@pbelevich
Update on "C++ API parity: at::Tensor::version"
6f6ab44 
Differential Revision: [D17427577](https://our.internmc.facebook.com/intern/diff/D17427577)

[ghstack-poisoned]
@pbelevich
Update on "C++ API parity: at::Tensor::version"
f447e39 
Differential Revision: [D17427577](https://our.internmc.facebook.com/intern/diff/D17427577)

[ghstack-poisoned]
@pbelevich
Update on "C++ API parity: at::Tensor::version"
2b3ff65 
Differential Revision: [D17427577](https://our.internmc.facebook.com/intern/diff/D17427577)

[ghstack-poisoned]
@pbelevich
Update on "C++ API parity: at::Tensor::version"
ddde65c 
Differential Revision: [D17427577](https://our.internmc.facebook.com/intern/diff/D17427577)

[ghstack-poisoned]
@yf225 yf225 mentioned this pull request Sep 20, 2019
Closed
14 tasks
pbelevich added a commit to pbelevich/pytorch that referenced this pull request Sep 20, 2019
@pbelevich
C++ API parity: at::Tensor::version
55690f2 
ghstack-source-id: 23a5cb8
Pull Request resolved: pytorch#26217
zdevito pushed a commit to zdevito/ATen that referenced this pull request Sep 20, 2019
@pbelevich @facebook-github-bot
C++ API parity: at::Tensor::version
e52dade 
Summary: Pull Request resolved: pytorch/pytorch#26217

Test Plan: Imported from OSS

Differential Revision: D17427577

Pulled By: pbelevich

fbshipit-source-id: e9b3e76ca44df883e3038b688dd7b930752d93a2
@pbelevich pbelevich reopened this Sep 20, 2019
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 20, 2019

@pbelevich merged this pull request in 1985219.

@pbelevich pbelevich closed this Sep 20, 2019
mingbowan pushed a commit to mingbowan/pytorch that referenced this pull request Sep 23, 2019
@pbelevich
C++ API parity: at::Tensor::version
76e4392 
Summary: Pull Request resolved: pytorch#26217

Test Plan: Imported from OSS

Differential Revision: D17427577

Pulled By: pbelevich

fbshipit-source-id: e9b3e76ca44df883e3038b688dd7b930752d93a2
@facebook-github-bot facebook-github-bot deleted the gh/pbelevich/5/head branch October 28, 2019 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yf225 yf225 yf225 approved these changes

Assignees

No one assigned

Labels

Merged module: autograd Related to torch.autograd, and the autograd engine in general module: cpp Related to C++ API module: internals Related to internal abstractions in c10 and ATen module: pybind Related to our Python bindings / interactions with other Python libraries

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@pbelevich @yf225 @facebook-github-bot @pytorchbot @mruberry