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.

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

GH-15058: [C++][Python] Native support for UUID #37298

Merged
merged 22 commits into from
Aug 26, 2024
Merged

GH-15058: [C++][Python] Native support for UUID #37298

merged 22 commits into from
Aug 26, 2024

Conversation

rok
Copy link
Member

@rok rok commented Aug 22, 2023

Rationale for this change

See #15058.
UUID datatype is common in throughout the ecosystem and Arrow as supporting it as a native type would reduce friction.

What changes are included in this PR?

This PR implements logic for Arrow canonical extension type in C++ and a Python wrapper.

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes, new extension type is added.

@github-actions
Copy link

⚠️ GitHub issue #15058 has been automatically assigned in GitHub to PR creator.

@mapleFU
Copy link
Member

mapleFU commented Aug 22, 2023

mentioned #15058 (comment) , do we need to remove that?

@rok rok force-pushed the 15058 branch 2 times, most recently from 184e96c to e738540 Compare August 22, 2023 10:35
@rok rok force-pushed the 15058 branch 8 times, most recently from 16329bc to a5283b6 Compare August 25, 2023 19:21
@arogozhnikov
Copy link

Key error: A type extension with name uuid already defined

Looks Ruby already got support for UUID?

@rok rok force-pushed the 15058 branch 2 times, most recently from 3baa0a0 to d8bfaf8 Compare October 27, 2023 15:55
@rok rok marked this pull request as ready for review October 27, 2023 16:13
@rok rok requested a review from kou as a code owner October 27, 2023 18:39
@rok
Copy link
Member Author

rok commented Aug 26, 2024

Ping @jorisvandenbossche

@pitrou pitrou merged commit 2328b6e into apache:main Aug 26, 2024
39 of 41 checks passed
@pitrou pitrou removed the awaiting change review Awaiting change review label Aug 26, 2024
@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Aug 26, 2024
@rok
Copy link
Member Author

rok commented Aug 26, 2024

Thanks everyone!

Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 2328b6e.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 24 possible false positives for unstable benchmarks that are known to sometimes produce them.

@jorisvandenbossche
Copy link
Member

Thanks a lot @rok!

As another possible follow-up, would we want to support inferring and converting uuid.UUID objects in the python->arrow conversion layer? (although not sure if that's something people would be waiting for)

@rok
Copy link
Member Author

rok commented Aug 27, 2024

@jorisvandenbossche that sounds convenient! I've opened #43855 and will get around to it if someone else doesn't sooner :).

mapleFU pushed a commit to mapleFU/arrow that referenced this pull request Sep 3, 2024
### Rationale for this change

See apache#15058.
UUID datatype is common in throughout the ecosystem and Arrow as supporting it as a native type would reduce friction.

### What changes are included in this PR?

This PR implements logic for Arrow canonical extension type in C++ and a Python wrapper.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes, new extension type is added.
* Closes: apache#15058

Authored-by: Rok Mihevc <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
khwilson pushed a commit to khwilson/arrow that referenced this pull request Sep 14, 2024
### Rationale for this change

See apache#15058.
UUID datatype is common in throughout the ecosystem and Arrow as supporting it as a native type would reduce friction.

### What changes are included in this PR?

This PR implements logic for Arrow canonical extension type in C++ and a Python wrapper.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes, new extension type is added.
* Closes: apache#15058

Authored-by: Rok Mihevc <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
ianmcook added a commit that referenced this pull request Sep 17, 2024
### Rationale for this change

UUID extension types were made canonical in #41299 and are getting
native support in C++ and Python in #37298. As such, it makes sense to
provide an alternative user-defined extension type as an example that is
unlikely to become a canonical extension type anytime soon.

After discussion in #43809, we determined a `RationalType` would make
sense.

Please note that this is a redo of #43849 as I made a blunder and
accidentally pushed a branch that was in a wonky state.

### What changes are included in this PR?

A change in several doc locations which reference a `UuidType` extension
type have been changed to a `RationalType`.

For consistency, this PR also changes single quotes (`''`) to double
quotes (`""`) throughout the Python examples that it modifies.

Also, seemingly unrelated to this change, some doctests began failing as
numpy changed the `repr` of `float16`'s between 1.x and 2.x. We have
updated the failing doctest so that it supports both styles.

### Are these changes tested?

These are documentation changes and `archery docker run
conda-python-docs` succeeds locally.

### Are there any user-facing changes?

No.

cc @ianmcook @rok 
* GitHub Issue: #43809

---------

Co-authored-by: Ian Cook <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Python] Native support for UUID
7 participants