remove trim option to fkeep! and related functions (#32972)

* remove trim option to fkeep! and related functions

* add NEWS entry

NEWS.md

@@ -91,6 +91,10 @@ Standard library changes
 
 #### SparseArrays
 * `lu!` accepts `UmfpackLU` as an argument to make use of its symbolic factorization.
+* The `trim` keyword argument for the functions `fkeep!`, `tril!`, `triu!`,
+  `droptol!`,`dropzeros!` and `dropzeros` has been removed in favour of always
+  trimming. Calling these with `trim=false` could result in invalid sparse
+  arrays.
 
 #### Dates
 

stdlib/SparseArrays/src/sparsematrix.jl

@@ -1274,7 +1274,7 @@ end
 ## fkeep! and children tril!, triu!, droptol!, dropzeros[!]
 
 """
-    fkeep!(A::AbstractSparseArray, f, trim::Bool = true)
+    fkeep!(A::AbstractSparseArray, f)
 
 Keep elements of `A` for which test `f` returns `true`. `f`'s signature should be
 
@@ -1283,8 +1283,7 @@ Keep elements of `A` for which test `f` returns `true`. `f`'s signature should b
 where `i` and `j` are an element's row and column indices and `x` is the element's
 value. This method makes a single sweep
 through `A`, requiring `O(size(A, 2), nnz(A))`-time for matrices and `O(nnz(A))`-time for vectors
-and no space beyond that passed in. If `trim` is `true`, this method trims `rowvals(A)` or `nonzeroinds(A)` and
-`nonzeros(A)` to length `nnz(A)` after dropping elements.
+and no space beyond that passed in.
 
 # Examples
 ```jldoctest
@@ -1328,50 +1327,40 @@ function fkeep!(A::AbstractSparseMatrixCSC, f, trim::Bool = true)
         Acolptr[Aj+1] = Awritepos
     end
 
-    # Trim A's storage if necessary and desired
-    if trim
-        Annz = Acolptr[end] - 1
-        if length(Arowval) != Annz
-            resize!(Arowval, Annz)
-        end
-        if length(Anzval) != Annz
-            resize!(Anzval, Annz)
-        end
-    end
+    # Trim A's storage if necessary
+    Annz = Acolptr[end] - 1
+    resize!(Arowval, Annz)
+    resize!(Anzval, Annz)
 
-    A
+    return A
 end
 
-tril!(A::AbstractSparseMatrixCSC, k::Integer = 0, trim::Bool = true) =
-    fkeep!(A, (i, j, x) -> i + k >= j, trim)
-triu!(A::AbstractSparseMatrixCSC, k::Integer = 0, trim::Bool = true) =
-    fkeep!(A, (i, j, x) -> j >= i + k, trim)
+tril!(A::AbstractSparseMatrixCSC, k::Integer = 0) =
+    fkeep!(A, (i, j, x) -> i + k >= j)
+triu!(A::AbstractSparseMatrixCSC, k::Integer = 0) =
+    fkeep!(A, (i, j, x) -> j >= i + k)
 
 """
-    droptol!(A::AbstractSparseMatrixCSC, tol; trim::Bool = true)
+    droptol!(A::AbstractSparseMatrixCSC, tol)
 
-Removes stored values from `A` whose absolute value is less than or equal to `tol`,
-optionally trimming resulting excess space from `rowvals(A)` and `nonzeros(A)` when `trim`
-is `true`.
+Removes stored values from `A` whose absolute value is less than or equal to `tol`.
 """
-droptol!(A::AbstractSparseMatrixCSC, tol; trim::Bool = true) =
-    fkeep!(A, (i, j, x) -> abs(x) > tol, trim)
+droptol!(A::AbstractSparseMatrixCSC, tol) =
+    fkeep!(A, (i, j, x) -> abs(x) > tol)
 
 """
-    dropzeros!(A::AbstractSparseMatrixCSC; trim::Bool = true)
+    dropzeros!(A::AbstractSparseMatrixCSC;)
 
-Removes stored numerical zeros from `A`, optionally trimming resulting excess space from
-`rowvals(A)` and `nonzeros(A)` when `trim` is `true`.
+Removes stored numerical zeros from `A`.
 
 For an out-of-place version, see [`dropzeros`](@ref). For
 algorithmic information, see `fkeep!`.
 """
-dropzeros!(A::AbstractSparseMatrixCSC; trim::Bool = true) = fkeep!(A, (i, j, x) -> !iszero(x), trim)
+dropzeros!(A::AbstractSparseMatrixCSC) = fkeep!(A, (i, j, x) -> !iszero(x))
 """
-    dropzeros(A::AbstractSparseMatrixCSC; trim::Bool = true)
+    dropzeros(A::AbstractSparseMatrixCSC;)
 
-Generates a copy of `A` and removes stored numerical zeros from that copy, optionally
-trimming excess space from the result's `rowval` and `nzval` arrays when `trim` is `true`.
+Generates a copy of `A` and removes stored numerical zeros from that copy.
 
 For an in-place version and algorithmic information, see [`dropzeros!`](@ref).
 
@@ -1389,7 +1378,7 @@ julia> dropzeros(A)
   [3, 3]  =  1.0
 ```
 """
-dropzeros(A::AbstractSparseMatrixCSC; trim::Bool = true) = dropzeros!(copy(A), trim = trim)
+dropzeros(A::AbstractSparseMatrixCSC) = dropzeros!(copy(A))
 
 ## Find methods
 

stdlib/SparseArrays/src/sparsevector.jl

@@ -1920,7 +1920,7 @@ function sort(x::SparseVector{Tv,Ti}; kws...) where {Tv,Ti}
     SparseVector(n,newnzind,newnzvals)
 end
 
-function fkeep!(x::SparseVector, f, trim::Bool = true)
+function fkeep!(x::SparseVector, f)
     n = length(x::SparseVector)
     nzind = nonzeroinds(x)
     nzval = nonzeros(x)
@@ -1939,43 +1939,35 @@ function fkeep!(x::SparseVector, f, trim::Bool = true)
         end
     end
 
-    # Trim x's storage if necessary and desired
-    if trim
-        x_nnz = x_writepos - 1
-        if length(nzind) != x_nnz
-            resize!(nzval, x_nnz)
-            resize!(nzind, x_nnz)
-        end
-    end
+    # Trim x's storage if necessary
+    x_nnz = x_writepos - 1
+    resize!(nzval, x_nnz)
+    resize!(nzind, x_nnz)
 
-    x
+    return x
 end
 
 """
-    droptol!(x::SparseVector, tol; trim::Bool = true)
+    droptol!(x::SparseVector, tol)
 
-Removes stored values from `x` whose absolute value is less than or equal to `tol`,
-optionally trimming resulting excess space from `nonzeroinds(x)` and `nonzeros(x)` when `trim`
-is `true`.
+Removes stored values from `x` whose absolute value is less than or equal to `tol`.
 """
-droptol!(x::SparseVector, tol; trim::Bool = true) = fkeep!(x, (i, x) -> abs(x) > tol, trim)
+droptol!(x::SparseVector, tol) = fkeep!(x, (i, x) -> abs(x) > tol)
 
 """
-    dropzeros!(x::SparseVector; trim::Bool = true)
+    dropzeros!(x::SparseVector)
 
-Removes stored numerical zeros from `x`, optionally trimming resulting excess space from
-`nonzeroinds(x)` and `nonzeros(x)` when `trim` is `true`.
+Removes stored numerical zeros from `x`.
 
 For an out-of-place version, see [`dropzeros`](@ref). For
 algorithmic information, see `fkeep!`.
 """
-dropzeros!(x::SparseVector; trim::Bool = true) = fkeep!(x, (i, x) -> !iszero(x), trim)
+dropzeros!(x::SparseVector) = fkeep!(x, (i, x) -> !iszero(x))
 
 """
-    dropzeros(x::SparseVector; trim::Bool = true)
+    dropzeros(x::SparseVector)
 
-Generates a copy of `x` and removes numerical zeros from that copy, optionally trimming
-excess space from the result's `nzind` and `nzval` arrays when `trim` is `true`.
+Generates a copy of `x` and removes numerical zeros from that copy.
 
 For an in-place version and algorithmic information, see [`dropzeros!`](@ref).
 
@@ -1993,7 +1985,7 @@ julia> dropzeros(A)
   [3]  =  1.0
 ```
 """
-dropzeros(x::SparseVector; trim::Bool = true) = dropzeros!(copy(x), trim = trim)
+dropzeros(x::SparseVector) = dropzeros!(copy(x))
 
 function copy!(dst::SparseVector, src::SparseVector)
     length(dst::SparseVector) == length(src::SparseVector) || throw(ArgumentError("Sparse vectors should have the same length for copy!"))

stdlib/SparseArrays/test/sparse.jl

@@ -1679,15 +1679,11 @@ end
         for Awithzeros in (Aposzeros, Anegzeros, Abothsigns)
             # Basic functionality / dropzeros!
             @test dropzeros!(copy(Awithzeros)) == A
-            @test dropzeros!(copy(Awithzeros), trim = false) == A
             # Basic functionality / dropzeros
             @test dropzeros(Awithzeros) == A
-            @test dropzeros(Awithzeros, trim = false) == A
             # Check trimming works as expected
             @test length(nonzeros(dropzeros!(copy(Awithzeros)))) == length(nonzeros(A))
             @test length(rowvals(dropzeros!(copy(Awithzeros)))) == length(rowvals(A))
-            @test length(nonzeros(dropzeros!(copy(Awithzeros), trim = false))) == length(nonzeros(Awithzeros))
-            @test length(rowvals(dropzeros!(copy(Awithzeros), trim = false))) == length(rowvals(Awithzeros))
         end
     end
     # original lone dropzeros test

stdlib/SparseArrays/test/sparsevector.jl

@@ -1168,15 +1168,11 @@ end
     for vwithzeros in (vposzeros, vnegzeros, vbothsigns)
         # Basic functionality / dropzeros!
         @test dropzeros!(copy(vwithzeros)) == v
-        @test dropzeros!(copy(vwithzeros), trim = false) == v
         # Basic functionality / dropzeros
         @test dropzeros(vwithzeros) == v
-        @test dropzeros(vwithzeros, trim = false) == v
         # Check trimming works as expected
         @test length(nonzeros(dropzeros!(copy(vwithzeros)))) == length(nonzeros(v))
         @test length(nonzeroinds(dropzeros!(copy(vwithzeros)))) == length(nonzeroinds(v))
-        @test length(nonzeros(dropzeros!(copy(vwithzeros), trim = false))) == length(nonzeros(vwithzeros))
-        @test length(nonzeroinds(dropzeros!(copy(vwithzeros), trim = false))) == length(nonzeroinds(vwithzeros))
     end
 end