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

Avoid eltype degrading to Union{} for empty map/broadcast #664

Merged
merged 1 commit into from
Sep 27, 2019

Conversation

c42f
Copy link
Member

@c42f c42f commented Sep 26, 2019

This reverts to using Core.Compiler.return_type for map/broadcast, but
only in the very restricted case that the output container is completely
empty.

This is consistent with the way that return_type is used in Base for
collect and broadcast for empty collections only.

Fixes #528 using the strategy from #528 (comment)

@c42f
Copy link
Member Author

c42f commented Sep 26, 2019

@tkoolen I think this finally fixes #528 in a way which is safe. Or at least, it's no worse than what Base already does for mapping an empty collection. If it's good enough for Base, I think it's good enough for us :-)

@tkoolen
Copy link
Contributor

tkoolen commented Sep 26, 2019

Thank you! Makes sense and looks good to me once tests pass.

This reverts to using Core.Compiler.return_type for map/broadcast, but
only in the very restricted case that the output container is completely
empty.

This is consistent with the way that return_type is used in Base for
collect and broadcast for empty collections only.
@c42f c42f force-pushed the cjf/fix-mapped-empty-eltype branch from 3fea0a6 to 76d506e Compare September 26, 2019 21:30
@c42f
Copy link
Member Author

c42f commented Sep 26, 2019

Eh, that's a lot of test failures! That's what I get for pushing right after a last minute rebase. Lucky we have CI to keep me honest :-D Let's see what it says now.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 81.561% when pulling 76d506e on cjf/fix-mapped-empty-eltype into ae78e52 on master.

@c42f c42f added the bugfix label Sep 27, 2019
@c42f c42f merged commit 5005108 into master Sep 27, 2019
@c42f c42f added this to the 1.0 milestone Sep 27, 2019
@c42f c42f deleted the cjf/fix-mapped-empty-eltype branch September 27, 2019 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding n x 0 matrices results in Union{} eltype
4 participants