Skip to content

add packaging to the runtime requirements #113154

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
pmeier wants to merge 1 commit into from
Closed

add packaging to the runtime requirements #113154

pmeier wants to merge 1 commit into from

Conversation

@pmeier
Copy link
Collaborator

@pmeier pmeier commented Nov 7, 2023

Forward fix for #113023 (comment).

@pmeier pmeier requested review from Skylion007 and vfdev-5 November 7, 2023 11:36
@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Nov 7, 2023
@pytorch-bot
Copy link

pytorch-bot bot commented Nov 7, 2023 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/113154

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 713515d with merge base c6f435b (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pmeier pmeier mentioned this pull request Nov 7, 2023
Closed
@pmeier pmeier requested review from atalman and malfet November 7, 2023 14:50
@pmeier pmeier mentioned this pull request Nov 7, 2023
Merged
@malfet
Copy link
Contributor

malfet commented Nov 8, 2023

This is probably fine, but why not use torch.version?

@pmeier
Copy link
Collaborator Author

pmeier commented Nov 8, 2023

@malfet I didn't look into the original patch, but rather only provided a fix given that the other patch is good apart from the missing dependency. @vfdev-5 is torch.version an option?

@vfdev-5
Copy link
Contributor

vfdev-5 commented Nov 9, 2023 •
edited
Loading

I have impression that torch.version is not sufficient as package is used to parse and compare version like below:

torch_cuda_ver = packaging.version.parse(torch.version.cuda)
if torch_cuda_ver < MIN_CUDA_VERSION:
...

Maybe, we can use TorchVersion:

pytorch/torch/torch_version.py

Lines 41 to 57 in addb8e2

class TorchVersion(str):
"""A string with magic powers to compare to both Version and iterables!
Prior to 1.10.0 torch.__version__ was stored as a str and so many did
comparisons against torch.__version__ as if it were a str. In order to not
break them we have TorchVersion which masquerades as a str while also
having the ability to compare against both packaging.version.Version as
well as tuples of values, eg. (1, 2, 1)
Examples:
Comparing a TorchVersion object to a Version object
TorchVersion('1.10.0a') > Version('1.10.0a')
Comparing a TorchVersion object to a Tuple object
TorchVersion('1.10.0a') > (1, 2) # 1.2
TorchVersion('1.10.0a') > (1, 2, 1) # 1.2.1
Comparing a TorchVersion object against a string
TorchVersion('1.10.0a') > '1.2'
TorchVersion('1.10.0a') > '1.2.1'
"""

@pmeier
Copy link
Collaborator Author

pmeier commented Nov 14, 2023

#113023 was reverted.

@pmeier pmeier closed this Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Skylion007 Skylion007 Skylion007 approved these changes

@vfdev-5 vfdev-5 Awaiting requested review from vfdev-5

@malfet malfet Awaiting requested review from malfet

@atalman atalman Awaiting requested review from atalman

Assignees

No one assigned

Labels

dependency issue open source topic: not user facing topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pmeier @malfet @vfdev-5 @Skylion007 @pytorchbot