Skip to content

GH-46613: [GLib] Add GArrowBaseListDataType #46615

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

Merged
merged 6 commits into from
May 29, 2025

Conversation

hiroyuki-sato
Copy link
Collaborator

@hiroyuki-sato hiroyuki-sato commented May 27, 2025

Rationale for this change

GLib should be able to use arrow::BaseListType.

What changes are included in this PR?

Add GArrowBaseListDataType and use it as a base class of GArrowListDataType.

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

@hiroyuki-sato hiroyuki-sato requested a review from kou as a code owner May 27, 2025 11:30
Copy link

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

@kou
Copy link
Member

kou commented May 27, 2025

Could you use this as the parent class of GArrowListDataType?

@hiroyuki-sato
Copy link
Collaborator Author

@kou Thank you for your review you meant the following. correct? If so, fixed

-G_DECLARE_DERIVABLE_TYPE(
-  GArrowListDataType, garrow_list_data_type, GARROW, LIST_DATA_TYPE, GArrowDataType)
+G_DECLARE_DERIVABLE_TYPE(GArrowListDataType,
+                         garrow_list_data_type,
+                         GARROW,
+                         LIST_DATA_TYPE,
+                         GArrowBaseListDataType)

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels May 28, 2025
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels May 28, 2025
@hiroyuki-sato
Copy link
Collaborator Author

Thanks! All suggested change applied.

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels May 29, 2025
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

@kou kou merged commit 5890232 into apache:main May 29, 2025
10 of 11 checks passed
@kou kou removed the awaiting changes Awaiting changes label May 29, 2025
@github-actions github-actions bot added the awaiting merge Awaiting merge label May 29, 2025
@hiroyuki-sato hiroyuki-sato deleted the topic/base-list-data-type branch May 29, 2025 07:54
Copy link

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

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

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.

2 participants