Skip to content

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

Merged
lidavidm merged 2 commits into
apache:masterfrom
anjakefala:arrow-17131
Aug 22, 2022
Merged

ARROW-17131: [Python] add StructType().field(): returns a field by name or index#13652
lidavidm merged 2 commits into
apache:masterfrom
anjakefala:arrow-17131

Conversation

@anjakefala

@anjakefala anjakefala commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

github-actions Bot commented Jul 19, 2022

Copy link
Copy Markdown

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

@github-actions

github-actions Bot commented Jul 19, 2022

Copy link
Copy Markdown

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

@github-actions

github-actions Bot commented Jul 19, 2022

Copy link
Copy Markdown

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

wjones127
wjones127 reviewed Jul 19, 2022
View reviewed changes

@wjones127 wjones127 left a comment

Copy link
Copy Markdown
Member

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.

Comment thread python/pyarrow/types.pxi Resolved
Comment thread python/pyarrow/types.pxi Resolved
jorisvandenbossche
jorisvandenbossche reviewed Jul 20, 2022
View reviewed changes

@jorisvandenbossche jorisvandenbossche left a comment

Copy link
Copy Markdown
Member

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.

Comment thread python/pyarrow/types.pxi 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

@lidavidm lidavidm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, just one question

Comment thread python/pyarrow/types.pxi Resolved
pitrou
pitrou reviewed Aug 4, 2022
View reviewed changes
Comment thread python/pyarrow/types.pxi Resolved
Comment thread python/pyarrow/types.pxi Resolved
Comment thread python/pyarrow/types.pxi Resolved
Comment thread python/pyarrow/types.pxi Resolved
@anjakefala
anjakefala requested review from lidavidm and pitrou August 8, 2022 20:31
@pitrou

pitrou commented Aug 9, 2022

Copy link
Copy Markdown
Member

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

@anjakefala

anjakefala commented Aug 9, 2022

Copy link
Copy Markdown
Contributor Author

@pitrou Done!

jorisvandenbossche
jorisvandenbossche reviewed Aug 17, 2022
View reviewed changes
Comment thread python/pyarrow/tests/test_types.py Resolved
@anjakefala

anjakefala commented Aug 17, 2022

Copy link
Copy Markdown
Contributor 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 jorisvandenbossche, lidavidm and pitrou August 17, 2022 20:33
@anjakefala
anjakefala requested review from jorisvandenbossche and lidavidm and removed request for jorisvandenbossche and lidavidm August 17, 2022 21:08
lidavidm
lidavidm approved these changes Aug 22, 2022
View reviewed changes

@lidavidm lidavidm left a comment

Copy link
Copy Markdown
Member

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

ursabot commented Aug 22, 2022

Copy link
Copy Markdown

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

@jorisvandenbossche jorisvandenbossche jorisvandenbossche left review comments
@pitrou pitrou pitrou left review comments
@lidavidm lidavidm lidavidm approved these changes
@wjones127 wjones127 wjones127 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