Skip to content

Fix iteration for SparsePolynomial #405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 26, 2022
Merged

Conversation

jishnub
Copy link
Member

@jishnub jishnub commented May 26, 2022

This PR does two main things:

  1. Change firstindex, lastindex and eachindex for SparsePolynomials to use the default fallback methods. This fixes Bug in sparse polynomial iteration #404.
  2. Change PolynomialKeys and PolynomialValues to subtype AbstractSet, which, in the latter's case, involves adding a type-parameter. This makes empty SparsePolynomials work correctly by fetching the eltype from the parent type.

After this PR:

julia> p = SparsePolynomial([0])
SparsePolynomial(0)

julia> collect(p) # errors on master
Int64[]

julia> p = SparsePolynomial([0,1]) #404
SparsePolynomial(x)

julia> collect(p)
2-element Vector{Int64}:
 0
 1

Aside from this, there are some clean-ups of the iteration functions, an optimized length method for SparsePolynomial and fix some tests. Finally, change == checks for nothing to ===, which is the recommended style.

While the changes are largely bug-fixes, I've bumped the minor version as there are some technically breaking changes.

jishnub added 4 commits May 26, 2022 16:09
Change firstindex and lastindex for sparse polynomials to span
the entire range of indices
@codecov
Copy link

codecov bot commented May 26, 2022

Codecov Report

Merging #405 (19eb252) into master (f6b7258) will decrease coverage by 0.13%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##           master     #405      +/-   ##
==========================================
- Coverage   83.16%   83.02%   -0.14%     
==========================================
  Files          23       23              
  Lines        2810     2793      -17     
==========================================
- Hits         2337     2319      -18     
- Misses        473      474       +1     
Impacted Files Coverage Δ
src/common.jl 88.69% <88.88%> (-0.65%) ⬇️
src/polynomials/Poly.jl 82.43% <100.00%> (-0.91%) ⬇️
src/polynomials/SparsePolynomial.jl 98.29% <100.00%> (-0.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f6b7258...19eb252. Read the comment docs.

@jverzani
Copy link
Member

Wow. Thanks. I had gotten half way here this AM, but didn't think to fix the iterators so that pairs works as I had wanted. Most appreciated.

@jverzani jverzani merged commit ddb9e57 into JuliaMath:master May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in sparse polynomial iteration
2 participants