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

ARROW-7921: [Go] Add Reset method to various components and clean up comments. #6430

Closed
wants to merge 9 commits into from

Conversation

richardartoul
Copy link
Contributor

The reset method allow the data structures to be re-used so they don't have to be allocated over and over again.

@github-actions
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@wesm
Copy link
Member

wesm commented Feb 17, 2020

Can you please open a JIRA issue for the benefit of our changelog?

@wesm wesm requested a review from sbinet February 17, 2020 11:28
@@ -30,7 +30,7 @@ const (
stringArrayMaximumCapacity = math.MaxInt32
)

// A type which represents an immutable sequence of variable-length UTF-8 strings.
// String is a type which represents an immutable sequence of variable-length UTF-8 strings.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// String is a type which represents an immutable sequence of variable-length UTF-8 strings.
// String represents an immutable sequence of variable-length UTF-8 strings.

@@ -24,7 +24,7 @@ import (
"github.com/apache/arrow/go/arrow/memory"
)

// A type which represents the memory and metadata for an Arrow array.
// Data is a type which represents the memory and metadata for an Arrow array.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Data is a type which represents the memory and metadata for an Arrow array.
// Data represents the memory and metadata of an Arrow array.

go/arrow/memory/buffer.go Show resolved Hide resolved
@sbinet
Copy link
Contributor

sbinet commented Feb 17, 2020

also, it would be nice to have a few tests for this new API.

@richardartoul
Copy link
Contributor Author

@sbinet I think I addressed all your feedback. I updated the method strings and added some tests. Not really sure what to do about the mutability thing (I left my thoughts on that) but I don't think it impacts correctness at all and the new methods are mostly optimizations to prevent extra allocations for people who want to eak out some extra perf in the hot path.

@richardartoul richardartoul changed the title [Go] Add Reset method to various components and clean up comments. ARROW-7921: [Go] Add Reset method to various components and clean up comments. Feb 22, 2020
@richardartoul
Copy link
Contributor Author

@wesm Opened a ticket!

@github-actions
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants