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

MINOR: [Documentation][C++][Python][R] Clarify docstrings around max_chunksize #40251

Merged
merged 2 commits into from
Feb 27, 2024

Conversation

amoeba
Copy link
Member

@amoeba amoeba commented Feb 26, 2024

Rationale for this change

A user ran into confusion over the units of the max_chunksize argument in PyArrow and didn't see any reason not to make the documentation more explicit.

What changes are included in this PR?

Just changes to docstrings.

Are these changes tested?

No, though I did go through every change to see if it was correct and I'm pretty sure it's right. Good to double-check during review though.

Are there any user-facing changes?

Just docs.

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Feb 26, 2024
Copy link
Member

@paleolimbot paleolimbot left a comment

Choose a reason for hiding this comment

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

Thanks! I think you may need to run devtools::document() for R, though.

@amoeba
Copy link
Member Author

amoeba commented Feb 26, 2024

Ah, yep. Thanks @paleolimbot. That's done now.

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Thanks!

@jorisvandenbossche jorisvandenbossche merged commit 06d841e into apache:main Feb 27, 2024
33 of 34 checks passed
@jorisvandenbossche jorisvandenbossche removed the awaiting merge Awaiting merge label Feb 27, 2024
@github-actions github-actions bot added the awaiting merge Awaiting merge label Feb 27, 2024
Copy link

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

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

zanmato1984 pushed a commit to zanmato1984/arrow that referenced this pull request Feb 28, 2024
…chunksize (apache#40251)

### Rationale for this change

A user [ran into confusion](https://lists.apache.org/thread/8ym8r1z5gys7dpcr8rw8dvjbkqc2lf7f) over the units of the `max_chunksize` argument in PyArrow and didn't see any reason not to make the documentation more explicit.

### What changes are included in this PR?

Just changes to docstrings.

### Are these changes tested?

No, though I did go through every change to see if it was correct and I'm pretty sure it's right. Good to double-check during review though.

### Are there any user-facing changes?

Just docs.

Authored-by: Bryce Mecum <[email protected]>
Signed-off-by: Joris Van den Bossche <[email protected]>
thisisnic pushed a commit that referenced this pull request Mar 8, 2024
…chunksize (#40251)

### Rationale for this change

A user [ran into confusion](https://lists.apache.org/thread/8ym8r1z5gys7dpcr8rw8dvjbkqc2lf7f) over the units of the `max_chunksize` argument in PyArrow and didn't see any reason not to make the documentation more explicit.

### What changes are included in this PR?

Just changes to docstrings.

### Are these changes tested?

No, though I did go through every change to see if it was correct and I'm pretty sure it's right. Good to double-check during review though.

### Are there any user-facing changes?

Just docs.

Authored-by: Bryce Mecum <[email protected]>
Signed-off-by: Joris Van den Bossche <[email protected]>
thisisnic pushed a commit to thisisnic/arrow that referenced this pull request Mar 8, 2024
…chunksize (apache#40251)

### Rationale for this change

A user [ran into confusion](https://lists.apache.org/thread/8ym8r1z5gys7dpcr8rw8dvjbkqc2lf7f) over the units of the `max_chunksize` argument in PyArrow and didn't see any reason not to make the documentation more explicit.

### What changes are included in this PR?

Just changes to docstrings.

### Are these changes tested?

No, though I did go through every change to see if it was correct and I'm pretty sure it's right. Good to double-check during review though.

### Are there any user-facing changes?

Just docs.

Authored-by: Bryce Mecum <[email protected]>
Signed-off-by: Joris Van den Bossche <[email protected]>
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.

4 participants