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

WIP: [Docs] Update extension type examples to not use UUID #43849

Closed
wants to merge 130 commits into from

Conversation

khwilson
Copy link
Contributor

@khwilson khwilson commented Aug 27, 2024

Superseded by #44120

In a previous version of the docs, a UuidType was discussed as an
extension type. However, this type has been promoted to a canonical
type, and so no longer is a good example of an extension type a
user may wish to create.

We replace UuidType in the docs with a RationalType
Copy link

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

@ianmcook
Copy link
Member

@github-actions crossbow submit preview-docs

Copy link

Revision: fde3215

Submitted crossbow builds: ursacomputing/crossbow @ actions-d5e0428a8b

Task Status
preview-docs GitHub Actions

@ianmcook
Copy link
Member

@khwilson I found one more docs page where there are a couple of mentions of UUID as an example of a user-defined extension type:

extension type can still handle the underlying data. For example a
16-byte UUID value could be embedded in ``FixedSizeBinary(16)``, and
implementations that do not have this extension type can still work
with the underlying binary values and pass along the
``custom_metadata`` in subsequent Arrow protocol messages.

* ``uuid`` represented as ``FixedSizeBinary(16)`` with empty metadata

Could you replace these with your rational example? Thanks

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Aug 27, 2024
python/pyarrow/types.pxi Outdated Show resolved Hide resolved
python/pyarrow/types.pxi Outdated Show resolved Hide resolved
python/pyarrow/types.pxi Outdated Show resolved Hide resolved
python/pyarrow/types.pxi Outdated Show resolved Hide resolved
python/pyarrow/types.pxi Outdated Show resolved Hide resolved
python/pyarrow/types.pxi Outdated Show resolved Hide resolved
@ianmcook
Copy link
Member

@khwilson thank you; this looks pretty good! I left a bunch of comments. They should be mostly be easy to address (except the bit about what "parameterized" means which might require some thinking about how best to explain it).

@github-actions github-actions bot removed the awaiting changes Awaiting changes label Aug 28, 2024
@khwilson
Copy link
Contributor Author

Hi all. My deepest apologies. I got myself into a bit of trouble syncing with master and accidentally rebased instead of merging. I will close this issue for now to avoid spamming everyone as there is no way I know to unsubscribe you automatically.

@khwilson khwilson closed this Sep 14, 2024
@ianmcook
Copy link
Member

No worries, it happens :) Tag me in the new PR and I will review

@ianmcook ianmcook changed the title GH-43809: [Docs] Update extension type examples to not use UUID WIP: [Docs] Update extension type examples to not use UUID Sep 17, 2024
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.