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.

Sign up for GitHub

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

ARROW-17131: [Python] add StructType().field(): returns a field by name or index #13652

Merged
merged 2 commits into from
Aug 22, 2022
Merged

ARROW-17131: [Python] add StructType().field(): returns a field by name or index #13652

merged 2 commits into from
Aug 22, 2022

Conversation

anjakefala
Copy link
Collaborator

No description provided.

@github-actions
Copy link

https://issues.apache.org/jira/browse/ARROW-17131

@github-actions
Copy link

⚠️ Ticket has no components in JIRA, make sure you assign one.

@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

wjones127
wjones127 reviewed Jul 19, 2022
View reviewed changes
Copy link
Member

@wjones127 wjones127 left a comment

Choose a reason for hiding this comment

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

This looks good. I had two suggestions, which are mostly curiosities about Cython.

python/pyarrow/types.pxi Outdated Show resolved Hide resolved
python/pyarrow/types.pxi Outdated Show resolved Hide resolved
jorisvandenbossche
jorisvandenbossche reviewed Jul 20, 2022
View reviewed changes
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 for the PR!

It's also fine if you prefer to leave it for another JIRA/PR, but so the one other data type that also uses the existing fields method (defined on the base class) is UnionType, which currently also implements __getitem__ to access that field. For consistency, it would probably be good to add a public field() method to UnionType as well.

python/pyarrow/types.pxi Outdated Show resolved Hide resolved
@anjakefala anjakefala force-pushed the arrow-17131 branch 2 times, most recently from 1a13534 to 10bc33e Compare July 20, 2022 18:55
lidavidm
lidavidm reviewed Aug 3, 2022
View reviewed changes
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

Thanks, just one question

python/pyarrow/types.pxi Show resolved Hide resolved
pitrou
pitrou reviewed Aug 4, 2022
View reviewed changes
python/pyarrow/types.pxi Outdated Show resolved Hide resolved
python/pyarrow/types.pxi Show resolved Hide resolved
python/pyarrow/types.pxi Show resolved Hide resolved
python/pyarrow/types.pxi Outdated Show resolved Hide resolved
@anjakefala anjakefala requested review from pitrou and lidavidm August 8, 2022 20:31
@pitrou
Copy link
Member

pitrou commented Aug 9, 2022

@anjakefala Can you take a look at the CI failures?

@anjakefala
Copy link
Collaborator Author

@pitrou Done!

@anjakefala
Copy link
Collaborator Author

Added the [] test case back =) https://github.com/apache/arrow/compare/231f2b3c4d93c0bd3e61288165d1847f83142bdb..add10bdc632f7a064120489fe37f3d950cd4683b

@anjakefala anjakefala requested review from jorisvandenbossche and lidavidm and removed request for lidavidm, pitrou and jorisvandenbossche August 17, 2022 20:33
@anjakefala anjakefala requested review from lidavidm and jorisvandenbossche and removed request for jorisvandenbossche and lidavidm August 17, 2022 21:08
lidavidm
lidavidm approved these changes Aug 22, 2022
View reviewed changes
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

LGTM, sorry for the delay!

@lidavidm lidavidm merged commit 8c17925 into apache:master Aug 22, 2022
@anjakefala anjakefala deleted the arrow-17131 branch August 22, 2022 15:27
@ursabot
Copy link

ursabot commented Aug 22, 2022

Benchmark runs are scheduled for baseline = fa33ca4 and contender = 8c17925. 8c17925 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.37%] test-mac-arm
[Failed ⬇️0.27% ⬆️0.27%] ursa-i9-9960x
[Finished ⬇️0.57% ⬆️0.04%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 8c17925a ec2-t3-xlarge-us-east-2
[Failed] 8c17925a test-mac-arm
[Failed] 8c17925a ursa-i9-9960x
[Finished] 8c17925a ursa-thinkcentre-m75q
[Finished] fa33ca40 ec2-t3-xlarge-us-east-2
[Finished] fa33ca40 test-mac-arm
[Failed] fa33ca40 ursa-i9-9960x
[Finished] fa33ca40 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

zagto pushed a commit to zagto/arrow that referenced this pull request Oct 7, 2022
@anjakefala @zagto
ARROW-17131: [Python] add StructType().field(): returns a field by na…
c774d5d 
…me or index (apache#13652)

Authored-by: anjakefala <[email protected]>
Signed-off-by: David Li <[email protected]>
@asfimport asfimport mentioned this pull request Aug 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pitrou pitrou pitrou left review comments

@jorisvandenbossche jorisvandenbossche jorisvandenbossche left review comments

@wjones127 wjones127 wjones127 approved these changes

@lidavidm lidavidm lidavidm approved these changes

Assignees
No one assigned
Labels
Component: Python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@anjakefala @pitrou @ursabot @lidavidm @jorisvandenbossche @wjones127