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-35264: [Python] Interchange protocol: test clean-up #35530

Merged

Conversation

AlenkaF
Copy link
Member

@AlenkaF AlenkaF commented May 10, 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 GH-35490: [Python] Interchange protocol: update tests for string and large_string #35504

@github-actions
Copy link

@AlenkaF
Copy link
Member Author

AlenkaF commented May 10, 2023

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

if Version(pd.__version__) < Version("1.5.0"):
pytest.skip("__dataframe__ added to pandas in 1.5.0")
def test_pandas_roundtrip_categorical():
if Version(pd.__version__) < Version("2.0.2"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the pandas version for skipping tests due to Column.size() bug in pandas match between tests test_pandas_roundtrip_string (v2.0.1) and test_pandas_roundtrip_categorical (v2.0.2)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! =) There are also bitmasks involved here, so I have to change the skip message 😊

Copy link
Member Author

Choose a reason for hiding this comment

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

I have, unfortunately, added this change in the rebasing process and is therefore not visible in the commit history.
The change can be seen here:

def test_pandas_roundtrip_categorical():
if Version(pd.__version__) < Version("2.0.2"):
pytest.skip("Bitmasks not supported in pandas interchange implementation")

@danepitkin
Copy link
Contributor

Added one comment, but besides that LGTM!

@jorisvandenbossche
Copy link
Member

This needs a rebase now I merged the other PR

@AlenkaF AlenkaF force-pushed the gh-35264-interchange-test-updates branch from b3dafde to 0e74ace Compare May 11, 2023 09:07
@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels May 11, 2023
@AlenkaF
Copy link
Member Author

AlenkaF commented May 11, 2023

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

@github-actions
Copy link

Revision: 0e74ace

Submitted crossbow builds: ursacomputing/crossbow @ actions-894d083930

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 marked this pull request as ready for review May 11, 2023 10:05
@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels May 11, 2023
@jorisvandenbossche jorisvandenbossche merged commit 14f9bf9 into apache:main May 11, 2023
13 of 14 checks passed
@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting changes Awaiting changes labels May 11, 2023
@AlenkaF AlenkaF deleted the gh-35264-interchange-test-updates branch May 12, 2023 14:29
@ursabot
Copy link

ursabot commented May 13, 2023

Benchmark runs are scheduled for baseline = b36ff71 and contender = 14f9bf9. 14f9bf9 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 ⬇️0.84% ⬆️0.06%] test-mac-arm
[Finished ⬇️0.25% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.39% ⬆️0.06%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 14f9bf92 ec2-t3-xlarge-us-east-2
[Finished] 14f9bf92 test-mac-arm
[Finished] 14f9bf92 ursa-i9-9960x
[Finished] 14f9bf92 ursa-thinkcentre-m75q
[Finished] b36ff71b ec2-t3-xlarge-us-east-2
[Finished] b36ff71b test-mac-arm
[Finished] b36ff71b ursa-i9-9960x
[Finished] b36ff71b 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

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
…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] Dataframe interchange protocol updates after pandas bug fixes
4 participants