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

support eltypes without fields #242

Closed
wants to merge 1 commit into from
Closed

Conversation

aplavin
Copy link
Member

@aplavin aplavin commented Aug 26, 2022

Alternative to #235: actually support StructArrays with eltypes that have no fields.

I regularly get bitten by stuff like StructArray([1, 2, 3]) returning empty arrays, and ask to consider merging one of these PRs. #235 makes it throw an error instead of a silently wrong behavior, and this PR makes no-field eltypes actually work.

@aplavin aplavin mentioned this pull request Aug 26, 2022
@piever
Copy link
Collaborator

piever commented Sep 2, 2022

Thanks for showing what the fix would look like. For now, I'm closing in favor of #235, but we might revisit if there are complaints on the "empty struct" error.

@JoaoAparicio
Copy link

As predicted here this breaks compatibility with IndexedTables. Ongoing JuliaData/IndexedTables.jl#308

@aplavin
Copy link
Member Author

aplavin commented Nov 25, 2023

Note that this PR wasn't merged into StructArrays.

What exactly breaks in IndexedTables? I don't see any details in the linked issue.
Also note that earlier behavior (before #235) of StructArrays without any fields didn't make any sense:

julia> StructArray([1,2])
0-element StructArray() with eltype Int64 with indices 1:0

@JoaoAparicio
Copy link

What exactly breaks in IndexedTables? I don't see any details in the linked issue.

Nothing breaks right now, because compat of StructArrays is still on 0.4.

Here I've done some work to bring compat to 0.5.

IndexedTables is still compatible with 0.6 only up to v0.6.12. However, it's not compatible with v0.6.13 and onwards. What breaks is some constructor, as mentioned here. If you're interested I can find you the stacktrace. If you want to reproduce yourself, you can go on my 0.5 branch, and bump compat to 0.6.

@JoaoAparicio
Copy link

Ok I've retrieved the stacktrace for you and placed it here

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.

3 participants