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-35490: [Python] Interchange protocol: update tests for string and large_string #35504

Merged

Conversation

AlenkaF
Copy link
Member

@AlenkaF AlenkaF commented May 9, 2023

Rationale for this change

In pandas version 2.0.1 the interchange protocol implementation has support for large strings. The tests on our side need to be updated accordingly.

What changes are included in this PR?

Changes in tests:

  • test_pandas_assertion_error_large_string removed
  • test_roundtrip_pandas_string updated

@github-actions
Copy link

github-actions bot commented May 9, 2023

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.

Do we still want to test normal string for older pandas? (because now the test combines both, and so doesn't test it for older versions?)

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels May 9, 2023
@AlenkaF
Copy link
Member Author

AlenkaF commented May 9, 2023

Oh yes, of course! Will correct 👍

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels May 9, 2023
@github-actions github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting change review Awaiting change review awaiting changes Awaiting changes labels May 10, 2023
@AlenkaF
Copy link
Member Author

AlenkaF commented May 10, 2023

@github-actions crossbow submit test-conda-python--pandas-

@github-actions
Copy link

Revision: 0179e1c

Submitted crossbow builds: ursacomputing/crossbow @ actions-d113313268

Task Status
test-conda-python-3.7-pandas-1.0 Github Actions
test-conda-python-3.7-pandas-latest Github Actions
test-conda-python-3.8-pandas-latest Github Actions
test-conda-python-3.8-pandas-nightly Github Actions
test-conda-python-3.9-pandas-upstream_devel Github Actions

@AlenkaF AlenkaF force-pushed the gh-35490-interchange-large-string branch from 0179e1c to 5f05027 Compare May 10, 2023 10:27
@AlenkaF
Copy link
Member Author

AlenkaF commented May 10, 2023

@github-actions crossbow submit test-conda-python--pandas-

@github-actions
Copy link

Revision: 5f05027

Submitted crossbow builds: ursacomputing/crossbow @ actions-6f1dfae313

Task Status
test-conda-python-3.7-pandas-1.0 Github Actions
test-conda-python-3.7-pandas-latest Github Actions
test-conda-python-3.8-pandas-latest Github Actions
test-conda-python-3.8-pandas-nightly Github Actions
test-conda-python-3.9-pandas-upstream_devel Github Actions

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels May 11, 2023
@jorisvandenbossche
Copy link
Member

Nightly failure is unrelated (but something else we should fix)

@jorisvandenbossche jorisvandenbossche merged commit dec4453 into apache:main May 11, 2023
12 checks passed
@AlenkaF AlenkaF deleted the gh-35490-interchange-large-string branch May 11, 2023 08:00
@AlenkaF
Copy link
Member Author

AlenkaF commented May 11, 2023

Nightly failure is unrelated (but something else we should fix)

I think this failure is connected to #34789?

@jorisvandenbossche
Copy link
Member

Yes, indeed, I am looking at that

jorisvandenbossche pushed a commit that referenced this pull request May 11, 2023
The diff is a bit confusing so I will add some notes here:

- `test_categorical_roundtrip` is not removed but renamed to `test_pandas_roundtrip_categorical` so that all tests checking `pyarrow` -> `pandas` -> `pyarrow` start with `test_pandas_roundtip_*`
- the skip for the `test_pandas_roundtrip_categorical` is removed
- `test_pandas_to_pyarrow_categorical_with_missing` is removed as the conversion for categorical with missing values is now checked in `test_pandas_roundtrip_categorical`
- `test_roundtrip_pandas_boolean` is removed and the check for boolean has been added to `test_pandas_roundtrip`
- `test_pandas_assertion_error_large_string` and `test_pandas_to_pyarrow_string_with_missing` are removed as the update for these is done separately in #35504
* Closes: #35264

Authored-by: Alenka Frim <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@ursabot
Copy link

ursabot commented May 11, 2023

Benchmark runs are scheduled for baseline = 11780b9 and contender = dec4453. dec4453 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
[Finished ⬇️2.21% ⬆️0.0%] test-mac-arm
[Finished ⬇️1.02% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.63% ⬆️0.06%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] dec44537 ec2-t3-xlarge-us-east-2
[Finished] dec44537 test-mac-arm
[Finished] dec44537 ursa-i9-9960x
[Finished] dec44537 ursa-thinkcentre-m75q
[Finished] 11780b96 ec2-t3-xlarge-us-east-2
[Finished] 11780b96 test-mac-arm
[Finished] 11780b96 ursa-i9-9960x
[Finished] 11780b96 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

@ursabot
Copy link

ursabot commented May 11, 2023

['Python', 'R'] benchmarks have high level of regressions.
test-mac-arm
ursa-i9-9960x

ArgusLi pushed a commit to Bit-Quill/arrow that referenced this pull request May 15, 2023
…g and large_string (apache#35504)

### Rationale for this change

In pandas version 2.0.1 the interchange protocol implementation has support for large strings. The tests on our side need to be updated accordingly.

### What changes are included in this PR?
Changes in tests:
- `test_pandas_assertion_error_large_string` removed
- `test_roundtrip_pandas_string ` updated
* Closes: apache#35490

Authored-by: Alenka Frim <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
ArgusLi pushed a commit to Bit-Quill/arrow that referenced this pull request May 15, 2023
…35530)

The diff is a bit confusing so I will add some notes here:

- `test_categorical_roundtrip` is not removed but renamed to `test_pandas_roundtrip_categorical` so that all tests checking `pyarrow` -> `pandas` -> `pyarrow` start with `test_pandas_roundtip_*`
- the skip for the `test_pandas_roundtrip_categorical` is removed
- `test_pandas_to_pyarrow_categorical_with_missing` is removed as the conversion for categorical with missing values is now checked in `test_pandas_roundtrip_categorical`
- `test_roundtrip_pandas_boolean` is removed and the check for boolean has been added to `test_pandas_roundtrip`
- `test_pandas_assertion_error_large_string` and `test_pandas_to_pyarrow_string_with_missing` are removed as the update for these is done separately in apache#35504
* Closes: apache#35264

Authored-by: Alenka Frim <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
rtpsw pushed a commit to rtpsw/arrow that referenced this pull request May 16, 2023
…g and large_string (apache#35504)

### Rationale for this change

In pandas version 2.0.1 the interchange protocol implementation has support for large strings. The tests on our side need to be updated accordingly.

### What changes are included in this PR?
Changes in tests:
- `test_pandas_assertion_error_large_string` removed
- `test_roundtrip_pandas_string ` updated
* Closes: apache#35490

Authored-by: Alenka Frim <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
rtpsw pushed a commit to rtpsw/arrow that referenced this pull request May 16, 2023
…35530)

The diff is a bit confusing so I will add some notes here:

- `test_categorical_roundtrip` is not removed but renamed to `test_pandas_roundtrip_categorical` so that all tests checking `pyarrow` -> `pandas` -> `pyarrow` start with `test_pandas_roundtip_*`
- the skip for the `test_pandas_roundtrip_categorical` is removed
- `test_pandas_to_pyarrow_categorical_with_missing` is removed as the conversion for categorical with missing values is now checked in `test_pandas_roundtrip_categorical`
- `test_roundtrip_pandas_boolean` is removed and the check for boolean has been added to `test_pandas_roundtrip`
- `test_pandas_assertion_error_large_string` and `test_pandas_to_pyarrow_string_with_missing` are removed as the update for these is done separately in apache#35504
* Closes: apache#35264

Authored-by: Alenka Frim <frim.alenka@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.

[Python] Interchange - test failing on MacOS builds
3 participants