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

Jump to bottom

Rename ArrayData::validate_dict_offsets to ArrayData::validate_values #1889

Merged
merged 1 commit into from Jun 16, 2022
Merged

Rename ArrayData::validate_dict_offsets to ArrayData::validate_values #1889

merged 1 commit into from Jun 16, 2022

Conversation

frolovdev
Copy link
Contributor

Which issue does this PR close?

Closes #1812

@github-actions github-actions bot added the arrow Changes to the arrow crate label Jun 16, 2022
@tustvold tustvold changed the title rename function Rename ArrayData::validate_dict_offsets to validate_values Jun 16, 2022
tustvold
tustvold approved these changes Jun 16, 2022
View reviewed changes
Copy link
Contributor

@tustvold tustvold left a comment •
edited

Choose a reason for hiding this comment

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

Thank you.

Fwiw we could add a deprecated validate_dictionary_offset that forwards to this, to avoid this being a breaking change. But I don't feel strongly, will leave open to give other reviewers a chance to weigh in

@tustvold tustvold added the api-change Changes to the arrow API label Jun 16, 2022
@alamb alamb changed the title Rename ArrayData::validate_dict_offsets to validate_values Rename ArrayData::validate_dict_offsets to ArrayData::validate_values Jun 16, 2022
alamb
alamb approved these changes Jun 16, 2022
View reviewed changes
Copy link
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.

I think this is a small change and I do think it would be nice to mark the API deprecated for at least one release

However, since #1871 already has breaking API changes and I don't think this is a widely used API, I think this is fine

@alamb
Copy link
Contributor

alamb commented Jun 16, 2022

Thanks @frolovdev

@tustvold tustvold merged commit ada7fb6 into apache:master Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alamb alamb alamb approved these changes

@tustvold tustvold tustvold approved these changes

Assignees
No one assigned
Labels
api-change Changes to the arrow API arrow Changes to the arrow crate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rename ArrayData::validate_dictionary_offset to ArrayData::validate_values
3 participants
@frolovdev @alamb @tustvold