Define shape of some truncated zips

[mschauer]

"Closes" #17928 (#17928 is closed, but should be open and would be closed... )

Zip truncates iterators to the shortest length. Currently, length reports the effective length if possible, but size gives a DimensionMismatch. For a zip iterator z with one-dimensional components (HasShape{1} components) defining the size as (length(z), ) and axes correspondingly allows fast collection and it is also what collect expects to be defined as the iterator itself has HasShape{1}. But it is not clear how to define the size of truncated higher-dimensional iterators, so this is spared out. This makes the PR rather small and it should be not-breaking.

#20499 was closed so I think truncating zips are a reality now, I added a remark to the doc string.

[martinholters]

Needs to updated because of #29238, but I think the general idea is right.

[mschauer]

@martinholters Are you willing to look into this? It is perhaps easier for you than for me to understand what has to be done to achieve the same with the new parametrization.

[martinholters]

I think this a relatively direct translation of your approach:

diff --git a/base/iterators.jl b/base/iterators.jl
index bebe8ac170..0d52ca4c2a 100644
--- a/base/iterators.jl
+++ b/base/iterators.jl
@@ -312,10 +312,11 @@ function _zip_min_length(is)
     end
 end
 _zip_min_length(is::Tuple{}) = nothing
-size(z::Zip) = _promote_shape(map(size, z.is)...)
-axes(z::Zip) = _promote_shape(map(axes, z.is)...)
-_promote_shape(a, b...) = promote_shape(a, _promote_shape(b...))
-_promote_shape(a) = a
+size(z::Zip) = mapreduce(size, _zip_promote_shape, z.is)
+axes(z::Zip) = mapreduce(axes, _zip_promote_shape, z.is)
+_zip_promote_shape((a,)::Tuple{OneTo}, (b,)::Tuple{OneTo}) = (intersect(a, b),)
+_zip_promote_shape((m,)::Tuple{Integer},(n,)::Tuple{Integer}) = (min(m,n),)
+_zip_promote_shape(a, b) = promote_shape(a, b)
 eltype(::Type{Zip{Is}}) where {Is<:Tuple} = _zip_eltype(Is)
 _zip_eltype(::Type{Is}) where {Is<:Tuple} =
     tuple_type_cons(eltype(tuple_type_head(Is)), _zip_eltype(tuple_type_tail(Is)))

Please feel free to go ahead and update your PR with this.

[mschauer]

Thank you! I updated the PR with your help.

[mschauer]

Maybe helpful as a summary:

Before

julia>  [i for i in zip(1:3, 2:5)]
ERROR: DimensionMismatch("dimensions must match")

julia> for i in  zip(1:3, 2:5); println(i);end
(1, 2)
(2, 3)
(3, 4)

Now

julia>  [i for i in zip(1:3, 2:5)]
3-element Array{Tuple{Int64,Int64},1}:
 (1, 2)
 (2, 3)
 (3, 4)

julia> for i in  zip(1:3, 2:5); println(i);end
(1, 2)
(2, 3)
(3, 4)

[mschauer]

Regarding the news, would this be considered a change, a fix, or an extension of functionality? The technical description of the change would be:

• The size of a Zip of 1-d iterators (i.e. those where Base.IteratorSize is
  Base.HasShape{1}()) is now (length(n),) where n is the length of the zipped iterator.

[mschauer]

Ping?

[mschauer]

I added a news item.

[mschauer]

When would be a good time for you to review this?

[AzamatB]

Bump. I was just bitten by this.

[mschauer]

Ping

[mschauer]

Closes #33340, #33168

[mschauer]

@JeffBezanson Please allow me to ping you on this.

[StefanKarpinski]

@maleadt, how do we run PkgEval on this?

[maleadt]

@nanosoldier runtests(ALL, vs = ":master")

I had to look it up as well... (It's only part of the README in JuliaCI/Nanosoldier.jl#64)

[nanosoldier]

Your test job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

[mschauer]

Looks good. Nanosoldier reports 7 packages failed tests only on the current version, 10 packages passed tests only on the current version. Failures are due to lack of random seeding or time outs.

[mschauer]

The release candidate for Julia v1.4.0 is out, does this mean this also won't make the cut for 1.4 anymore??

[StefanKarpinski]

https://discourse.julialang.org/t/julia-1-4-feature-freeze-dec-15th/30453

[mschauer]

I see, so maybe 1.5.

[AzamatB]

Bump

[mschauer]

Bump

[StefanKarpinski]

If CI passes I'm going to merge. This hasn't gotten discussed, but my general approach with stalled PRs that can't get dev attention is to merge so as to prevent passive stonewalling.

[rfourquet]

maybe add a test for axes ?

[rfourquet]

maybe add a test for axes ?

[mschauer]

I incorporated @rfourquet ’s suggestions

[rfourquet]

Thank you @mschauer and sorry this took so long to get merged!