Skip to content
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

Avoided string comparisons for PyTorch version comparison #4307

Merged
merged 8 commits into from
Oct 11, 2021
Merged

Avoided string comparisons for PyTorch version comparison #4307

merged 8 commits into from
Oct 11, 2021

Conversation

ABD-01
Copy link
Contributor

@ABD-01 ABD-01 commented Aug 23, 2021

Fixes #4296

NicolasHug
NicolasHug reviewed Aug 23, 2021
View reviewed changes
setup.py Show resolved Hide resolved
@fmassa
Copy link
Member

fmassa commented Sep 1, 2021

Waiting on pytorch/pytorch#63848 to be merged before we merge this

@ABD-01
Copy link
Contributor Author

ABD-01 commented Sep 2, 2021

Waiting on pytorch/pytorch#63848 to be merged before we merge this

Cool. Thanks

@nivedwho
Copy link

@fmassa The other PR is merged and TorchVersion should work now. But this PR does not use TorchVersion and hence does not really address the issue you mentioned right?

@ABD-01
Copy link
Contributor Author

ABD-01 commented Sep 12, 2021

It uses torchversion, with new methods __lt__(),__ge__() and __le__() this will work perfect when performing operation: 'greater than equals to' which currently is done by splitting str and comparing the converted ints.

datumbox
datumbox approved these changes Oct 11, 2021
View reviewed changes
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

LGTM. The version object clearly supports string comparisons and the PR on core was merged, so I think it's safe to merge this one.

@datumbox datumbox merged commit 221dd5f into pytorch:main Oct 11, 2021
@ABD-01
Copy link
Contributor Author

ABD-01 commented Oct 11, 2021

Thank You 😊

@ABD-01 ABD-01 deleted the version_compare branch October 12, 2021 07:26
facebook-github-bot pushed a commit that referenced this pull request Oct 14, 2021
@prabhat00155 @facebook-github-bot
[fbsync] Updated setup.py to use TorchVersion object for version comp…
fd58d13 
…arision (#4307)

Reviewed By: fmassa

Differential Revision: D31649964

fbshipit-source-id: 670d26caf53ffc9127ed22afc74cec8ec5641595
mszhanyi pushed a commit to mszhanyi/vision that referenced this pull request Oct 19, 2021
@ABD-01 @mszhanyi
Updated setup.py to use TorchVersion object for version comparision (p…
ae22fa4 
…ytorch#4307)
cyyever pushed a commit to cyyever/vision that referenced this pull request Nov 16, 2021
@ABD-01 @cyyever
Updated setup.py to use TorchVersion object for version comparision (p…
53a76e3 
…ytorch#4307)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fmassa fmassa fmassa left review comments

@NicolasHug NicolasHug NicolasHug left review comments

@vadimkantorov vadimkantorov vadimkantorov left review comments

@datumbox datumbox datumbox approved these changes

Assignees
No one assigned
Labels
ciflow/default cla signed enhancement topic: build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Avoid string comparisons for PyTorch version comparison
7 participants
@ABD-01 @fmassa @nivedwho @vadimkantorov @NicolasHug @datumbox @facebook-github-bot