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-43809: [Docs] Update extension type examples to not use UUID #44120

Merged
merged 19 commits into from
Sep 17, 2024

Conversation

khwilson
Copy link
Contributor

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

@khwilson
Copy link
Contributor Author

@ianmcook, apologies again for accidentally pushing my wonky branch.

The two remaining issues with the build were (1) some spacing issues arising from my IDE's formatter and copy/pasting outputs from terminals; and (b) Numpy changed the repr for float16's in 2.x so the half-float test in types.pxi was sometimes failing depending on which version of numpy was installed. I have modified this test so that it works both with numpy 1.x and 2.x.

@jonkeane
Copy link
Member

@github-actions crossbow submit preview-docs

Copy link

Revision: ff67035

Submitted crossbow builds: ursacomputing/crossbow @ actions-51ff65abfb

Task Status
preview-docs GitHub Actions

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Sep 16, 2024
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Sep 16, 2024
@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Sep 17, 2024
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Sep 17, 2024
Copy link
Member

@ianmcook ianmcook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @khwilson! This looks great. I will merge this soon unless @rok, @jorisvandenbossche, or others have any further comments.

@ianmcook
Copy link
Member

@github-actions crossbow submit preview-docs

Copy link

Revision: 0502ff8

Submitted crossbow builds: ursacomputing/crossbow @ actions-8955adf366

Task Status
preview-docs GitHub Actions

@ianmcook ianmcook merged commit 0d4badb into apache:main Sep 17, 2024
14 checks passed
@ianmcook
Copy link
Member

Thanks again @khwilson!

Copy link

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

There were no benchmark performance regressions. 🎉

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

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.

3 participants