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

Tweak ibis.array() docstring, improve test, add xfail test #8023

Closed

Conversation

NickCrews
Copy link
Contributor

@NickCrews NickCrews commented Jan 18, 2024

adds an xfail test for #8022

I discovered that actually fixing the problem wasn't trivial, as I wasn't sure about the ibis internals and how to create an ArrayScalar object correctly. But still I think this is an improvement.

Not totally in love with how I patched ibis.options.repr.interactive.max_length, open to suggestions.

Also discovered that pandas and dask can't handle mixed scalars and columns, so I fixed that.

Copy link
Contributor

ACTION NEEDED

Ibis follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message.

Please update your PR title and description to match the specification.

pd.concat and dd.concat can only handle array-likes.
If we are given a scalar, we need to convert it to a Series.
@NickCrews NickCrews force-pushed the array_scalar_from_scalar branch from a53d244 to 5fa5125 Compare January 19, 2024 01:30
@cpcloud
Copy link
Member

cpcloud commented Jan 20, 2024

I think the abstraction is a bit off for construction of array literals. We shouldn't be constructing an ArrayColumn simply because the inputs to array are expressions, but instead do something like we're doing with map and struct: use the shape of the inputs to determine the output shape of the literal value.

@cpcloud
Copy link
Member

cpcloud commented Jan 20, 2024

@NickCrews If you don't mind I'm going to cherry pick the first two commits here and apply them to #8049!

@cpcloud cpcloud closed this Jan 20, 2024
@NickCrews
Copy link
Contributor Author

NickCrews commented Jan 20, 2024

you're right, using .shape makes way more sense. that new PR looks like a better move!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants