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

fix(api): use to_pyarrow() instead of execute() when pretty printing scalars #10186

Merged
merged 3 commits into from
Sep 23, 2024

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Sep 20, 2024

Closes #10185.

@github-actions github-actions bot added the tests Issues or PRs related to tests label Sep 20, 2024
@cpcloud
Copy link
Member Author

cpcloud commented Sep 20, 2024

Right, we have some backends that actually go through pandas to get to pyarrow.

repr(table.limit(1).string_col)

# execute doesn't get called
execute_spy.assert_not_called()
Copy link
Member

Choose a reason for hiding this comment

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

Right, we have some backends that actually go through pandas to get to pyarrow.

If things pass otherwise, the fix seems fine to me without this test at all. No need to assert we're not calling execute when reading the formatting code it's clear we go through to_pyarrow.

Copy link
Member Author

Choose a reason for hiding this comment

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

The main point of this test was to make it explicit what happens and in this case make us aware of any backends that have coupling between the pandas and pyarrow format converters. I think I'll leave the test, with appropriate xfails, as a form of documentation.

@cpcloud cpcloud added this to the 10.0 milestone Sep 23, 2024
@jcrist jcrist merged commit 10b38ee into ibis-project:main Sep 23, 2024
76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chore: scalar printing in interactive mode uses execute() but other types use to_pyarrow()
2 participants