Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-89083: add support for UUID version 8 (RFC 9562)#123224
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.
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
picnixz commented Aug 22, 2024 • edited by github-actions bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by github-actions bot
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
vstinner left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change mostly LGTM, but there is missing documentation in uuid.rst ("TODO" ;-)).
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
vstinner commented Sep 19, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
By the way, |
picnixz commented Sep 19, 2024
I had a little interrogation here. Should we use keyword-only parameters (if so, which names? with/without the I also wondered whether to keep |
vstinner commented Sep 19, 2024
I'm fine with |
| b = random.getrandbits(12) | ||
| if c is None: | ||
| import random | ||
| c = random.getrandbits(62) |
vstinnerSep 26, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense / is it possible to reject values outside the expected value range? For example, reject negative numbers?
Maybe something like:
orig_a = a a &= 0xffff_ffff_ffff if a != orig_a: raise ValueError("...") I don't know. Would it be consistent with other uuid functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UUIDv1 does not reject them so I wouldn't worry too much about it. v3 and v5 are not based on integral inputs.
vstinnerSep 26, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with having the same behavior than uuid1() in this case, since it's documented.
vstinner left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but I would prefer to have a second core dev to review the feature.
hugovk commented Nov 2, 2024
(What's New conflict resolved) |
hugovk left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Hugo van Kemenade <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
picnixz commented Nov 11, 2024
Thanks Hugo for the review! I've changed the docstrings in tests to regular comments to avoid cluttering the output in verbose mode and used |
vstinner commented Nov 12, 2024
Merged, thanks @picnixz. |
) Co-authored-by: Hugo van Kemenade <[email protected]>
) Co-authored-by: Hugo van Kemenade <[email protected]>
Simplest implementation of UUIDv8. Extensible implementation where hash functions can be used may be a follow-up PR / feature.
Entropy considerations can be discussed directly on the PR since the issue is becoming larger and larger (but we could also consider opening a separate issue to discuss the implementation more in details).
📚 Documentation preview 📚: https://cpython-previews--123224.org.readthedocs.build/