Avoid calling extrema(...; dims) with empty arrays#1711
Avoid calling extrema(...; dims) with empty arrays#1711joa-quim merged 1 commit intoGenericMappingTools:masterfrom
extrema(...; dims) with empty arrays#1711Conversation
There's a proposed "minor change" to Julia v1.13 that would make `extrema([], dims=2)` (and other such reductions) an error, akin to how `extrema([])` is an error when `init` is not specified. This package was flagged as having (at least) one such usage here; it errored in [PkgEval](https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/813bcf3_vs_c3e7b1b/report.html). See JuliaLang/julia#55628 for more details. This addresses that first such usage; there may be [other places](https://github.com/search?q=repo%3AGenericMappingTools%2FGMT.jl+%2F%28%3F%3Aextrema%7Cminimum%7Cmaximum%29.*dims%2F&type=code) that similarly need to change by either supplying `init` or skipping the empty case entirely.
joa-quim
left a comment
There was a problem hiding this comment.
Thanks for letting me know about this. But I have a doubt on how to handle this. For example in this case, the fix would better be 4 lines above, after the declaration of
dest::Matrix{Float64} = zeros(Float64, DS.n_rows, DS.n_columns + plusZzero)
to add
isempty(dest) && continue # Skip empty segments
But, and here is my doubt, would this type of solution avoid that nanosoldier would still flag the current case?
|
Yeah, that sounds like a great solution that would further simply things here; I'm not familiar with this codebase so I made the most minimal and straightforward change I could easily see. I do suspect that it'll also happen in those other places I linked; PkgEval errored while precompiling the package so we only tripped over the very first failure. |
|
Hmm, wait, I'm a bit confused. You say that |
joa-quim
left a comment
There was a problem hiding this comment.
OK, the fix needs to be here after all (skipping above would break some code)
|
Try to address all of these cases with #1713 |
|
Thanks for taking this across the line — I know changes like these are annoying, and I appreciate the understanding! |
There's a proposed "minor change" to Julia v1.13 that would make
extrema([], dims=2)(and other such reductions) an error, akin to howextrema([])is an error wheninitis not specified. This package was flagged as having (at least) one such usage here; it errored in PkgEval. See JuliaLang/julia#55628 for more details.This addresses that first such usage; there may be other places that similarly need to change by either supplying
initor skipping the empty case entirely.