-
Notifications
You must be signed in to change notification settings - Fork 805
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
Add uuid python convertions #4836
Conversation
I'm currently stuck on the issue with the tests and I'm not sure how to fix the code for the error in |
To bless the ui tests, you can either run |
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! Couple of initial thoughts...
src/conversions/uuid.rs
Outdated
} else if let Ok(py_bytearray) = obj.downcast::<PyByteArray>() { | ||
&py_bytearray.to_vec() |
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 would suggest we might want to stay away from Bytearray::to_vec
(see #4736). Maybe for the moment just avoid byte arrays? Or maybe we want to go through the buffer protocol? That's not really much better 🤔
src/conversions/uuid.rs
Outdated
|
||
fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> { | ||
let uuid_cls = get_uuid_cls(py)?; | ||
let kwargs = [("int", self.as_u128())].into_py_dict(py)?; |
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.
We are much more efficient calling as positional arguments, it might be worth benchmarking that here. (I guess the tuple would be something like (None, None, None, None, int)
.)
* Fix struct layouts on GraalPy * Add changelog item
* fix: cross-compilation compatibility checks for Windows * add newsfragments * add simple test
* Expand docs on when and why allow_threads is necessary * spelling * simplify example a little * use less indirection in the example * Update guide/src/parallelism.md * Add note about the GIL preventing parallelism * Update guide/src/free-threading.md Co-authored-by: Bruno Kolenbrander <[email protected]> * pared down text about need to use with_gil * rearrange slightly --------- Co-authored-by: Bruno Kolenbrander <[email protected]>
* clarify safety docs for PyDict API * get dict size using PyDict_Size on free-threaded build * avoid unnecessary critical sections in PyDict * add changelog entry * fix build error on GIL-enabled build * address code review comments * move attribute below doc comment * ignore unsafe_op_in_unsafe_fn in next_unchecked --------- Co-authored-by: David Hewitt <[email protected]>
* implement locked iteration for PyList * fix limited API and PyPy support * fix formatting of safety docstrings * only define fold and rfold on not(feature = "nightly") * add missing try_fold implementation on nightly * Use split borrows for locked iteration for PyList Inline ListIterImpl implementations by using split borrows and destructuring let Self { .. } = self destructuring inside BoundListIterator impls. Signed-off-by: Manos Pitsidianakis <[email protected]> * use a function to do the split borrow * add changelog entries * fix clippy on limited API and PyPy * use a macro for the split borrow * add a test that mutates the list during a fold * enable next_unchecked on PyPy * fix incorrect docstring for locked_for_each * simplify borrows by adding BoundListIterator::with_critical_section * fix build on GIL-enabled and limited API builds * fix docs build on MSRV --------- Signed-off-by: Manos Pitsidianakis <[email protected]> Co-authored-by: Manos Pitsidianakis <[email protected]>
I made a mistake with my Git manipulations; I'll open another PR, sorry |
You can also force push to this branch to fix your mistake without having to open a second PR. |
Add conversions to/from UUID #4365