Skip to content

Conversation

@vtjnash
Copy link
Member

@vtjnash vtjnash commented Sep 9, 2021

Generalization of #41919
Fixes #42168

Needed to also fix some ssair code, which was unnecessarily relying on inference recursively, and led to some other fixes.

@vtjnash vtjnash added backport 1.6 Change should be backported to release-1.6 backport 1.7 labels Sep 9, 2021
@KristofferC
Copy link
Member

This requires a manual backport against backports-release-1.6.

@bramtayl
Copy link
Contributor

This seems to have broken Unzip.jl. Is the type assert really necessary?

julia> using Unzip

julia> function unstable(x)
           if x == 2
               (x, x + 0.0, x, x + 0.0)
           else
               (x, x + 0.0)
           end
       end
unstable (generic function with 1 method)

julia> unzip(Iterators.map(unstable, 1:3))
ERROR: TypeError: in typeassert, expected AbstractVector{<:Tuple{Int64, Float64, Vararg{Real}}}, got a value of type Unzip.Rows{Tuple{Int64, Float64, Union{Missing, Int64}, Union{Missing, Float64}}, 1, Tuple{Vector{Int64}, Vector{Float64}, Vector{Union{Missing, Int64}}, Vector{Union{Missing, Float64}}}}
Stacktrace:
 [1] _collect(c::Unzip.Rows{Tuple{}, 1, Tuple{}}, itr::Base.Generator{UnitRange{Int64}, typeof(unstable)}, #unused#::Base.EltypeUnknown, isz::Base.HasShape{1})
   @ Base .\array.jl:754
 [2] unzip(rows::Base.Generator{UnitRange{Int64}, typeof(unstable)})
   @ Unzip C:\Users\brand\.julia\dev\Unzip\src\Unzip.jl:282
 [3] top-level scope
   @ REPL[14]:1

@vtjnash
Copy link
Member Author

vtjnash commented Sep 1, 2022

It looks like Unzip has a bug where it accidentally tries to return a Missing value in the result, where no such value existed initially?

@bramtayl
Copy link
Contributor

bramtayl commented Sep 2, 2022

Nope, that's not a bug, it's a choice. Unzip will fill in missing for iterators of differently sized tuples:

unzip([(1, 'a'), (2,), (3, 'b')] = ([1, 2, 3], [1, missing, 'b'])

I found this while rewriting, so this is no longer an issue, but.

@vtjnash
Copy link
Member Author

vtjnash commented Sep 2, 2022

zip is expected to truncate to the shortest input

@bramtayl
Copy link
Contributor

bramtayl commented Sep 2, 2022

Hmm, well, I guess that implies that zip could never provide tuples of different sizes, so unzipping tuples of different sizes is undefined...I could error here, which would make the code a lot simpler but also less fun. As of now, I have my own version of the collect method, without a type assert, that will error for empty iterators if I can't guess the column types. https://github.com/bramtayl/Unzip.jl/blob/c239114b4b7faf71dab924c1832a11eaa0312d22/src/Unzip.jl#L317

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 1.6 Change should be backported to release-1.6

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Problematic interaction between Stateful and map

5 participants