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

GH-36809: [Python] MapScalar.as_py with custom field name #36830

Merged

Conversation

0x26res
Copy link
Contributor

@0x26res 0x26res commented Jul 23, 2023

Rationale for this change

MapScalar.as_py doesn't take into account custom key/value field names

What changes are included in this PR?

Fix and tests

Are these changes tested?

Simple unit test

Are there any user-facing changes?

No API changes.

@github-actions
Copy link

⚠️ GitHub issue #36809 has been automatically assigned in GitHub to PR creator.

@kou kou changed the title GH-36809: Python MapScalar as_py with custom field name GH-36809: [Python] MapScalar.as_py with custom field name Jul 24, 2023
@kou
Copy link
Member

kou commented Jul 24, 2023

Could you revert updating submodules?
It seems that we don't need them.

@0x26res
Copy link
Contributor Author

0x26res commented Jul 24, 2023

Could you revert updating submodules? It seems that we don't need them.

I agree, I didn't intend to do that. Do you know which git command I need to run to revert them?

@kou
Copy link
Member

kou commented Jul 24, 2023

The following command lines will work:

cd cpp/submodules/parquet-testing
git checkout b2e7cc755159196e3a068c8594f7acbaecfdaaac
cd -
cd testing
git checkout 47f7b56b25683202c1fd957668e13f2abafc0f12
cd -
git add cpp/submodules/parquet-testing testing
git commit
git push

@0x26res
Copy link
Contributor Author

0x26res commented Jul 24, 2023

That worked, thanks 👍

@0x26res
Copy link
Contributor Author

0x26res commented Jul 26, 2023

@kou do you mind reviewing again? Thanks.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

@kou kou merged commit 52ac718 into apache:main Jul 26, 2023
11 checks passed
@kou kou removed the awaiting review Awaiting review label Jul 26, 2023
@github-actions github-actions bot added the awaiting merge Awaiting merge label Jul 26, 2023
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 52ac718.

There was 1 benchmark result indicating a performance regression:

The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.

R-JunmingChen pushed a commit to R-JunmingChen/arrow that referenced this pull request Aug 20, 2023
…he#36830)

### Rationale for this change

`MapScalar.as_py` doesn't take into account custom key/value field names

### What changes are included in this PR?

Fix and tests 

### Are these changes tested?

Simple unit test

### Are there any user-facing changes?

No API changes.
* Closes: apache#36809

Authored-by: aandres <aandres@tradewelltech.co>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…he#36830)

### Rationale for this change

`MapScalar.as_py` doesn't take into account custom key/value field names

### What changes are included in this PR?

Fix and tests 

### Are these changes tested?

Simple unit test

### Are there any user-facing changes?

No API changes.
* Closes: apache#36809

Authored-by: aandres <aandres@tradewelltech.co>
Signed-off-by: Sutou Kouhei <kou@clear-code.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.

[Python] KeyError in MapScalar with custom field name
2 participants