remove single-argument methods for map, foreach, Iterators.map
#52631
Conversation
nsajko
force-pushed
the
empty_map
branch
4 times, most recently
from
a514e3d to
4f5da74
Compare
|
The test failures seem unrelated (they're system-specific). |
|
I guess this should get a Nanosoldier PkgEval run (the issue this fixes is marked as breaking). |
nsajko
added
the
needs pkgeval
In the case of `map` and `foreach`, this removes awkward functionality. `Iterators.map(::Any)`, on the other hand, already throwed anyway. Fixes JuliaLang#35293
|
Fixed conflict. |
|
@nanosoldier |
|
The package evaluation job you requested has completed - possible new issues were detected. |
nsajko
removed
the
needs pkgeval
|
I, for one, support this change. LGTM. |
|
@nsajko if you can analyze the errors and report whether they're real, that would likely help decide whether this is feasible. |
LilithHafner
added
minor change
There was a problem hiding this comment.
Given widespread approval, limited opposition, triage approval, and a nanosoldier run that, according to @nsajko's detailed analysis, revealed no real breakages, this LGTM from a breakage standpoint.
The implementation looks complete and sufficiently tested.
Single-argument `map` is being removed from Julia after an analysis of registered packages found that the change doesn't break the functionality of any package. See JuliaLang/julia#35293 and JuliaLang/julia#52631 That said, the Julia change breaks your test suite, so here's a PR that fixes that.
LilithHafner
removed
the
merge me
|
Should |
|
I think they should have also been included. If someone makes a PR soon I think those removals should be backported to 1.11 so they land with this one. As long as they are not widely used, I oppose the existence of those methods on the same grounds as |
Those weren't part of the linked issue, so presumably they're not covered with the triage decision. A new issue should be opened IMO, and discussed by triage I guess. |
|
No need to get triage approval to open a PR! (plus, we need a PR to run nanosoldier which will inform decisionmaking) |
|
(Though I would want to bring this back to triage before merging such a PR) |
In case this is something you actually want this works: julia> using InfiniteArrays
julia> f = () -> 1
#5 (generic function with 1 method)
julia> [f() for _ = 1:∞]
#7.(ℵ₀-element InfiniteArrays.InfUnitRange{Int64} with indices OneToInf()) with indices OneToInf():
1
1
1
1
1
1
1
1
1
⋮ |
PR JuliaLang#52631 made `map` throw `MethodError` when called without an iterator argument. That made `mapreduce` throw `MethodError` when called without an iterator argument, too, something that was *not* noticed back then. This change makes iteratorless `mapreduce` throw before it can be called, instead of when it tries to call `map` without passing it an iterator argument. After this change all of these functions should only have methods that take a positive number of iterator arguments: 1. `map` 2. `Iterators.map` 3. `foreach` 4. `reduce` 5. `foldl` 6. `foldr` 7. `mapreduce` 8. `mapfoldl` 9. `mapfoldr`
PR JuliaLang#52631 made `map` throw `MethodError` when called without an iterator argument. That made `mapreduce` throw `MethodError` when called without an iterator argument, too, something that was *not* noticed back then. This change makes iteratorless `mapreduce` throw before it can be called, instead of when it tries to call `map` (or `Generator`) without passing it an iterator argument. Now all of these functions should only have methods that take a positive number of iterator arguments, which improves consistency: 1. `map` 2. `Iterators.map` 3. `foreach` 4. `reduce` 5. `foldl` 6. `foldr` 7. `mapreduce` 8. `mapfoldl` 9. `mapfoldr` 10. `Base.Generator`
|
Followup: #54088 |
PR #52631 made `map` throw `MethodError` when called without an iterator argument. That made `mapreduce` throw `MethodError` when called without an iterator argument, too, something that was *not* noticed back then. This change makes iteratorless `mapreduce` throw before it can be called, instead of when it tries to call `map` (or `Generator`) without passing it an iterator argument. Now all of these functions should only have methods that take a positive number of iterator arguments, which improves consistency: 1. `map` 2. `Iterators.map` 3. `foreach` 4. `reduce` 5. `foldl` 6. `foldr` 7. `mapreduce` 8. `mapfoldl` 9. `mapfoldr` 10. `Base.Generator`
In the case of
mapandforeach, this removes awkward functionality.Iterators.map(::Any), on the other hand, already throwed anyway.Fixes #35293