Skip to content

Commit

Permalink
fix: make compose do what it says (#1606)
Browse files Browse the repository at this point in the history 
* fix: make compose do what it says

- First step at resolving oscar-system/Oscar.jl#690
- Introduce a mandatory keyword `inner = :first/:second` argument for
  compose(f, g) and add a hint for f(g)/g(f) syntax.

[breaking]

* chore: bump to 0.39.0
  • Loading branch information
@thofma
thofma authored Feb 13, 2024
1 parent e2292c2 commit d65c125
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 57 deletions.
2 changes: 1 addition & 1 deletion Project.toml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "AbstractAlgebra"
uuid = "c3fe647b-3220-5bb0-a1ea-a7954cac585d"
version = "0.38.1"
version = "0.39.0"

[deps]
InteractiveUtils = "b77e0a4c-d291-57a0-90e8-8db25a27a240"
Expand Down
2 changes: 1 addition & 1 deletion docs/src/polynomial.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ julia> k = evaluate(f, 3)
julia> m = evaluate(f, x^2 + 2x + 1)
x^5 + 4*x^4 + 7*x^3 + 7*x^2 + 4*x + 4
julia> n = compose(f, g)
julia> n = compose(f, g; inner = :second)
(x^3 + 2*x^2 + x)*y^2 + (2*x^5 + 2*x^4 + 4*x^3 + 9*x^2 + 6*x + 1)*y + x^7 + 4*x^5 + 5*x^4 + 5*x^3 + 10*x^2 + 8*x + 5
julia> p = subst(f, M)
Expand Down
26 changes: 21 additions & 5 deletions src/AbsSeries.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -764,13 +764,29 @@ end
###############################################################################

@doc raw"""
compose(a::AbsPowerSeriesRingElem, b::AbsPowerSeriesRingElem)
compose(f::AbsPowerSeriesRingElem, g::AbsPowerSeriesRingElem; inner)
Compose the series $a$ with the series $b$ and return the result,
i.e. return $a\circ b$. The two series do not need to be in the same ring,
however the series $b$ must have positive valuation or an exception is raised.
Compose the series $a$ with the series $b$ and return the result.
- If `inner = :second`, then `f(g)` is returned and `g` must have positive valuation.
- If `inner = :first`, then `g(f)` is returned and `f` must have positive valuation.
"""
function compose(a::AbsPowerSeriesRingElem, b::AbsPowerSeriesRingElem)
function compose(f::AbsPowerSeriesRingElem, g::AbsPowerSeriesRingElem; inner = nothing)
if inner === nothing
error("""compose(f, g) requires a keyword argument inner
- inner = :second yields f(g)
- inner = :first yields g(f)
Alternatively, use directly f(g) or g(f)
""")
end

if inner == :second
return _compose_right(f, g)
else
return _compose_right(g, f)
end
end

function _compose_right(a::AbsPowerSeriesRingElem, b::AbsPowerSeriesRingElem)
valuation(b) == 0 && error("Series being substituted must have positive valuation")
i = length(a)
R = base_ring(a)
Expand Down
29 changes: 24 additions & 5 deletions src/Poly.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2201,12 +2201,31 @@ function evaluate(a::PolyRingElem, b::T) where T <: RingElement
end

@doc raw"""
compose(a::PolyRingElem, b::PolyRingElem)
compose(f::PolyRingElem, g::PolyRingElem; inner)
Compose the polynomial $a$ with the polynomial $b$ and return the result.
- If `inner = :right`, then `f(g)` is returned.
- If `inner = :left`, then `g(f)` is returned.
"""
function compose(f::PolyRingElem, g::PolyRingElem; inner = nothing)
if inner === nothing
error("""compose(f, g) requires a keyword argument inner
- inner = :second yields f(g)
- inner = :first yields g(f)
Alternatively, use directly f(g) or g(f)
""")
end

if inner == :second
return _compose_right(f, g)
elseif inner == :first
return _compose_right(g, f)
else
error("Keyword inner must be :first or :second")
end
end

Compose the polynomial $a$ with the polynomial $b$ and return the result,
i.e. return $a\circ b$.
"""
function compose(a::PolyRingElem, b::PolyRingElem)
function _compose_right(a::PolyRingElem, b::PolyRingElem)
i = length(a)
R = base_ring(a)
S = parent(b)
Expand Down
26 changes: 21 additions & 5 deletions src/RelSeries.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -991,13 +991,29 @@ end
###############################################################################

@doc raw"""
compose(a::RelPowerSeriesRingElem, b::RelPowerSeriesRingElem)
compose(f::RelPowerSeriesRingElem, g::RelPowerSeriesRingElem; inner)
Compose the series $a$ with the series $b$ and return the result,
i.e. return $a\circ b$. The two series do not need to be in the same ring,
however the series $b$ must have positive valuation or an exception is raised.
Compose the series $a$ with the series $b$ and return the result.
- If `inner = :second`, then `f(g)` is returned and `g` must have positive valuation.
- If `inner = :first`, then `g(f)` is returned and `f` must have positive valuation.
"""
function compose(a::RelPowerSeriesRingElem, b::RelPowerSeriesRingElem)
function compose(f::RelPowerSeriesRingElem, g::RelPowerSeriesRingElem; inner = nothing)
if inner === nothing
error("""compose(f, g) requires a keyword argument inner
- inner = :second yields f(g)
- inner = :first yields g(f)
Alternatively, use directly f(g) or g(f)
""")
end

if inner == :second
return _compose_right(f, g)
else
return _compose_right(g, f)
end
end

function _compose_right(a::RelPowerSeriesRingElem, b::RelPowerSeriesRingElem)
valuation(b) == 0 && error("Series being substituted must have positive valuation")
i = pol_length(a)
R = base_ring(a)
Expand Down
2 changes: 1 addition & 1 deletion src/polysubst.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ for T in subtypes(PolyRingElem)
if parent(f) != parent(a)
return subst(f, a)
end
return compose(f, a)
return compose(f, a; inner = :second)
end

(f::T)(a::Integer) = evaluate(f, a)
Expand Down
36 changes: 18 additions & 18 deletions test/generic/AbsSeries-test.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -783,10 +783,10 @@ end

g = rand(R, 1:10, -10:10)

@test compose(f1 + f2, g) == compose(f1, g) + compose(f2, g)
@test compose(x, g) == g
@test compose(x^2, g) == g^2
@test compose(R(), g) == R()
@test compose(f1 + f2, g; inner = :second) == compose(f1, g; inner = :second) + compose(f2, g; inner = :second)
@test compose(x, g; inner = :second) == g
@test compose(x^2, g; inner = :second) == g^2
@test compose(R(), g; inner = :second) == R()
end

S, y = power_series_ring(ZZ, 10, "y", model=:capped_absolute)
Expand All @@ -796,8 +796,8 @@ end

g = rand(S, 1:10, -10:10)

@test compose(f1 + f2, g) == compose(f1, g) + compose(f2, g)
@test compose(R(), g) == S()
@test compose(f1 + f2, g; inner = :second) == compose(f1, g; inner = :second) + compose(f2, g; inner = :second)
@test compose(R(), g; inner = :second) == S()
end

# Inexact field
Expand All @@ -808,10 +808,10 @@ end

g = rand(R, 1:10, -10:10)

@test isapprox(compose(f1 + f2, g), compose(f1, g) + compose(f2, g))
@test isapprox(compose(x, g), g)
@test isapprox(compose(x^2, g), g^2)
@test isapprox(compose(R(), g), R())
@test isapprox(compose(f1 + f2, g; inner = :second), compose(f1, g; inner = :second) + compose(f2, g; inner = :second))
@test isapprox(compose(x, g; inner = :second), g)
@test isapprox(compose(x^2, g; inner = :second), g^2)
@test isapprox(compose(R(), g; inner = :second), R())
end

S, y = power_series_ring(RealField, 10, "y", model=:capped_absolute)
Expand All @@ -820,8 +820,8 @@ end
f2 = rand(R, 0:10, -10:10)

g = rand(S, 1:10, -10:10)
@test isapprox(compose(f1 + f2, g), compose(f1, g) + compose(f2, g))
@test isapprox(compose(R(), g), S())
@test isapprox(compose(f1 + f2, g; inner = :second), compose(f1, g; inner = :second) + compose(f2, g; inner = :second))
@test isapprox(compose(R(), g; inner = :second), S())
end

# Non-integral domain
Expand All @@ -833,10 +833,10 @@ end

g = rand(R, 1:10, -10:10)

@test compose(f1 + f2, g) == compose(f1, g) + compose(f2, g)
@test compose(x, g) == g
@test compose(x^2, g) == g^2
@test compose(R(), g) == R()
@test compose(f1 + f2, g; inner = :second) == compose(f1, g; inner = :second) + compose(f2, g; inner = :second)
@test compose(x, g; inner = :second) == g
@test compose(x^2, g; inner = :second) == g^2
@test compose(R(), g; inner = :second) == R()
end

S, y = power_series_ring(T, 10, "y", model=:capped_absolute)
Expand All @@ -846,8 +846,8 @@ end

g = rand(S, 1:10, -10:10)

@test compose(f1 + f2, g) == compose(f1, g) + compose(f2, g)
@test compose(R(), g) == S()
@test compose(f1 + f2, g; inner = :second) == compose(f1, g; inner = :second) + compose(f2, g; inner = :second)
@test compose(R(), g; inner = :second) == S()
end
end

Expand Down
8 changes: 5 additions & 3 deletions test/generic/Poly-test.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1847,7 +1847,9 @@ end
g = rand(R, -1:5, -10:10)
h = rand(R, -1:5, -10:10)

@test compose(f, compose(g, h)) == compose(compose(f, g), h)
@test compose(f, compose(g, h; inner = :second); inner = :second) == compose(compose(f, g; inner = :second), h; inner = :second)
@test compose(f, g; inner = :first) == g(f)
@test compose(f, g; inner = :second) == f(g)
end

# Inexact field
Expand All @@ -1858,7 +1860,7 @@ end
g = rand(R, -1:5, 0:1)
h = rand(R, -1:5, 0:1)

@test isapprox(compose(f, compose(g, h)), compose(compose(f, g), h))
@test isapprox(compose(f, compose(g, h; inner = :second); inner = :second), compose(compose(f, g; inner = :second), h; inner = :second))
end

# Non-integral domain
Expand All @@ -1870,7 +1872,7 @@ end
g = rand(R, -1:5, 0:5)
h = rand(R, -1:5, 0:5)

@test compose(f, compose(g, h)) == compose(compose(f, g), h)
@test compose(f, compose(g, h; inner = :second); inner = :second) == compose(compose(f, g; inner = :second), h; inner = :second)
end
end

Expand Down
36 changes: 18 additions & 18 deletions test/generic/RelSeries-test.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -806,10 +806,10 @@ end

g = rand(R, 1:10, -10:10)

@test compose(f1 + f2, g) == compose(f1, g) + compose(f2, g)
@test compose(x, g) == g
@test compose(x^2, g) == g^2
@test compose(R(), g) == R()
@test compose(f1 + f2, g; inner = :second) == compose(f1, g; inner = :second) + compose(f2, g; inner = :second)
@test compose(x, g; inner = :second) == g
@test compose(x^2, g; inner = :second) == g^2
@test compose(R(), g; inner = :second) == R()
end

S, y = power_series_ring(ZZ, 10, "y")
Expand All @@ -819,8 +819,8 @@ end

g = rand(S, 1:10, -10:10)

@test compose(f1 + f2, g) == compose(f1, g) + compose(f2, g)
@test compose(R(), g) == S()
@test compose(f1 + f2, g; inner = :second) == compose(f1, g; inner = :second) + compose(f2, g; inner = :second)
@test compose(R(), g; inner = :second) == S()
end

# Inexact field
Expand All @@ -831,10 +831,10 @@ end

g = rand(R, 1:10, -10:10)

@test isapprox(compose(f1 + f2, g), compose(f1, g) + compose(f2, g))
@test isapprox(compose(x, g), g)
@test isapprox(compose(x^2, g), g^2)
@test isapprox(compose(R(), g), R())
@test isapprox(compose(f1 + f2, g; inner = :second), compose(f1, g; inner = :second) + compose(f2, g; inner = :second))
@test isapprox(compose(x, g; inner = :second), g)
@test isapprox(compose(x^2, g; inner = :second), g^2)
@test isapprox(compose(R(), g; inner = :second), R())
end

S, y = power_series_ring(RealField, 10, "y")
Expand All @@ -843,8 +843,8 @@ end
f2 = rand(R, 0:10, -10:10)

g = rand(S, 1:10, -10:10)
@test isapprox(compose(f1 + f2, g), compose(f1, g) + compose(f2, g))
@test isapprox(compose(R(), g), S())
@test isapprox(compose(f1 + f2, g; inner = :second), compose(f1, g; inner = :second) + compose(f2, g; inner = :second))
@test isapprox(compose(R(), g; inner = :second), S())
end

# Non-integral domain
Expand All @@ -856,10 +856,10 @@ end

g = rand(R, 1:10, -10:10)

@test compose(f1 + f2, g) == compose(f1, g) + compose(f2, g)
@test compose(x, g) == g
@test compose(x^2, g) == g^2
@test compose(R(), g) == R()
@test compose(f1 + f2, g; inner = :second) == compose(f1, g; inner = :second) + compose(f2, g; inner = :second)
@test compose(x, g; inner = :second) == g
@test compose(x^2, g; inner = :second) == g^2
@test compose(R(), g; inner = :second) == R()
end

S, y = power_series_ring(T, 10, "y")
Expand All @@ -869,8 +869,8 @@ end

g = rand(S, 1:10, -10:10)

@test compose(f1 + f2, g) == compose(f1, g) + compose(f2, g)
@test compose(R(), g) == S()
@test compose(f1 + f2, g; inner = :second) == compose(f1, g; inner = :second) + compose(f2, g; inner = :second)
@test compose(R(), g; inner = :second) == S()
end
end

Expand Down

2 comments on commit d65c125

@thofma
Copy link
Member Author

@thofma thofma commented on d65c125 Feb 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JuliaRegistrator register

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registration pull request created: JuliaRegistries/General/100795

Tip: Release Notes

Did you know you can add release notes too? Just add markdown formatted text underneath the comment after the text
"Release notes:" and it will be added to the registry PR, and if TagBot is installed it will also be added to the
release that TagBot creates. i.e.

@JuliaRegistrator register

Release notes:

## Breaking changes

- blah

To add them here just re-invoke and the PR will be updated.

Tagging

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v0.39.0 -m "<description of version>" d65c12553fc31cbb670609aae3b1c9b8449aa7f0
git push origin v0.39.0

Please sign in to comment.