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-17320: [Python] Refine pyarrow.parquet API exposure #14096

Merged
merged 7 commits into from Sep 27, 2022
Merged

ARROW-17320: [Python] Refine pyarrow.parquet API exposure #14096

merged 7 commits into from Sep 27, 2022

Conversation

milesgranger
Copy link
Contributor

@milesgranger milesgranger commented Sep 12, 2022

Fixes ARROW-17320

Added a deprecation for _filters_to_expression -> filters_to_expression, in c7fdff3 let me know if that commit should be dropped. :)

@github-actions
Copy link

@AlenkaF
Copy link
Member

AlenkaF commented Sep 12, 2022

The only comment I would have is about the linter error. If you have seen it already, please ignore me - I have missed this error in the past and so am adding this just in case.

filters_to_expression now need some extra docstrings besides the description:

INFO:archery:Running Python docstring linters

pyarrow.parquet.core.filters_to_expression
PR01: Parameters {'filters'} not documented

@milesgranger
Copy link
Contributor Author

Ah, thanks for that! 👍 I suppose now that it's public, it raises the lint error. I'll wait until it's confirmed we ought to deprecate it in the first place. I was thinking if it was suppose to go away with the legacy dataset, then might not want to rename it just to remove it later. :)

@AlenkaF
Copy link
Member

AlenkaF commented Sep 12, 2022

I suppose now that it's public, it raises the lint error.

Yes, exactly.

I'll wait until it's confirmed we ought to deprecate it in the first place. I was thinking if it was suppose to go away with the legacy dataset, then might not want to rename it just to remove it later. :)

Makes sense. Let's see what the decision will be - otherwise it could be renamed for a while I guess.

@jorisvandenbossche
Copy link
Member

There is still a doc lint error:

pyarrow.parquet.core.filters_to_expression
PR01: Parameters {'filters'} not documented

(the Dev lint failure seems unrelated)

@milesgranger
Copy link
Contributor Author

@jorisvandenbossche I think it's okay now, the one failure doesn't seem related.

@jorisvandenbossche jorisvandenbossche merged commit 4d0fbff into apache:master Sep 27, 2022
@milesgranger milesgranger deleted the ARROW-17320_parquet-api-exposure branch September 27, 2022 09:18
@ursabot
Copy link

ursabot commented Sep 27, 2022

Benchmark runs are scheduled for baseline = df7babb and contender = 4d0fbff. 4d0fbff is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.68% ⬆️0.0%] test-mac-arm
[Failed ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.11% ⬆️0.04%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 4d0fbffd ec2-t3-xlarge-us-east-2
[Failed] 4d0fbffd test-mac-arm
[Failed] 4d0fbffd ursa-i9-9960x
[Finished] 4d0fbffd ursa-thinkcentre-m75q
[Finished] df7babbf ec2-t3-xlarge-us-east-2
[Finished] df7babbf test-mac-arm
[Failed] df7babbf ursa-i9-9960x
[Finished] df7babbf ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

fatemehp pushed a commit to fatemehp/arrow that referenced this pull request Oct 17, 2022
Fixes [ARROW-17320](https://issues.apache.org/jira/browse/ARROW-17320?page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel&focusedCommentId=17577330)

Added a deprecation for `_filters_to_expression` -> `filters_to_expression`, in apache@c7fdff3 let me know if that commit should be dropped. :) 

Authored-by: Miles Granger <miles59923@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
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.

None yet

4 participants