Skip to content

chore: Make binary_mut kernel accept different type for second arg#5833

Merged
tustvold merged 1 commit intoapache:masterfrom
viirya:diff_type_binary_mut
Jun 3, 2024
Merged

chore: Make binary_mut kernel accept different type for second arg#5833
tustvold merged 1 commit intoapache:masterfrom
viirya:diff_type_binary_mut

Conversation

@viirya
Copy link
Copy Markdown
Member

@viirya viirya commented Jun 2, 2024

Which issue does this PR close?

Closes #5818.

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

@github-actions github-actions Bot added the arrow Changes to the arrow crate label Jun 2, 2024
@tustvold tustvold added the api-change Changes to the arrow API label Jun 2, 2024
@tustvold
Copy link
Copy Markdown
Contributor

tustvold commented Jun 2, 2024

This is technically an API change as it could effect type inference

@viirya
Copy link
Copy Markdown
Member Author

viirya commented Jun 2, 2024

Thank you @tustvold

@tustvold tustvold merged commit 198af7a into apache:master Jun 3, 2024
Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @viirya

Comment thread arrow-arith/src/arity.rs
/// This function gives error of original [`PrimitiveArray`] `a` if it is not a mutable
/// primitive array.
pub fn binary_mut<T, F>(
pub fn binary_mut<T, U, F>(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FWIW I have added a doc example (and thus test coverage) for this change in #5798

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thank you @alamb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-change Changes to the arrow API arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

binary_mut kernel requires both args to be the same type (which is inconsistent with binary)

3 participants