Skip to content
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.

Sign up for GitHub

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

Jump to bottom

pretty printing for maps #1424

Merged
merged 5 commits into from
Sep 13, 2023
Merged

pretty printing for maps #1424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9253a2a
pretty printing for FunctionalMap
simonbrandhorst Sep 12, 2023
20f0c6d
pretty printing for more map types
simonbrandhorst Sep 12, 2023
b59a4dd
Apply suggestions from code review
simonbrandhorst Sep 13, 2023
ae60200
Apply suggestions from code review
simonbrandhorst Sep 13, 2023
0c9e15a
more adjustments
fingolfin Sep 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions docs/src/functional_map.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,9 @@ $R$ and $S$ corresponding to the Julia function $f$.

```jldoctest
julia> f = map_from_func(x -> x + 1, ZZ, ZZ)
Map with the following data

Domain:
=======
Integers

Codomain:
========
Integers
Map defined by a Julia function
from integers
to integers

julia> f(ZZ(2))
3
Expand Down
12 changes: 3 additions & 9 deletions docs/src/map_cache.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,9 @@ Caches can also be turned on and off at run time (see below).

```jldoctest
julia> f = map_from_func(x -> x + 1, ZZ, ZZ)
Map with the following data

Domain:
=======
Integers

Codomain:
========
Integers
Map defined by a Julia function
from integers
to integers

julia> g = cached(f);

Expand Down
44 changes: 12 additions & 32 deletions docs/src/map_with_inverse.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,9 @@ codomain of $f$.

```jldoctest
julia> f = map_with_retraction_from_func(x -> x + 1, x -> x - 1, ZZ, ZZ)
Map with retraction with the following data

Domain:
=======
Integers

Codomain:
========
Integers
Map with retraction
from integers
to integers

julia> a = f(ZZ(1))
2
Expand Down Expand Up @@ -101,33 +95,19 @@ the second two functions return the corresponding second maps.

```jldoctest
julia> f = map_with_retraction_from_func(x -> x + 1, x -> x - 1, ZZ, ZZ)
Map with retraction with the following data

Domain:
=======
Integers

Codomain:
========
Integers
Map with retraction
from integers
to integers

julia> g = inv(f)
Map with section with the following data

Domain:
=======
Integers

Codomain:
========
Integers
Map with section
from integers
to integers

julia> h = f*g
Composite map consisting of the following

Integers -> Integers
then
Integers -> Integers
Composite map
first map: integers -> integers
next map: integers -> integers

julia> a = h(ZZ(1))
1
Expand Down
10 changes: 1 addition & 9 deletions docs/src/residue.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,7 @@ julia> k = S(x + 1)
x + 1

julia> U, f = quo(R, x^3 + 3x + 1)
(Residue ring of univariate polynomial ring modulo x^3 + 3*x + 1, Map with section with the following data

Domain:
=======
Univariate polynomial ring in x over rationals

Codomain:
========
Residue ring of univariate polynomial ring modulo x^3 + 3*x + 1)
(Residue ring of univariate polynomial ring modulo x^3 + 3*x + 1, Map: univariate polynomial ring -> residue ring)

julia> U === S
true
Expand Down
123 changes: 84 additions & 39 deletions src/generic/Map.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,35 @@
return f.map2(f.map1(a))::elem_type(C)
end

function show_short(io::IO, M::CompositeMap)
show_short(io, M.map1)
println(io, "then")
print(io, M.map2)
function show(io::IO, ::MIME"text/plain", M::CompositeMap)
io = pretty(io)
println(io, "Composite map")
println(io, Indent(), "first ", Lowercase(), map1(M))
h = map2(M)
while h isa FunctionalCompositeMap || h isa CompositeMap
println(io, "next ", Lowercase(), h)
h = map2(h)
end

Check warning on line 30 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L28-L30 

Added lines #L28 - L30 were not covered by tests
print(io, "next ", Lowercase(), h, Dedent())
end

function show(io::IO, M::CompositeMap)
println(io, "Composite map consisting of the following")
println(io, "")
show_short(io, M.map1)
println(io, "then")
show_short(io, M.map2)
if get(io, :supercompact, false)

Check warning on line 35 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L35 

Added line #L35 was not covered by tests
# no nested printing
print(io, "Composite map")

Check warning on line 37 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L37 

Added line #L37 was not covered by tests
else
io = pretty(io)
print(io, "Map: ", Lowercase(), domain(M))
h = map2(M)
while h isa FunctionalCompositeMap || h isa CompositeMap
print(io, " -> ", Lowercase(), domain(h))
h = map2(h)
end
print(io, " -> ", Lowercase(), codomain(M))

Check warning on line 46 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L39-L46 

Added lines #L39 - L46 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

Seem this function is not tested at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe. Our coverage of show methods isn't great. But then there are various opinions on whether/how these should be tested.

Copy link
Member

Choose a reason for hiding this comment

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

There are? I thought this is pretty clear: they are tested via doctests in suitable places? That was actually the motivation for PR #1431 -- with that merged, more of the code in this PR here will be tested :-).

But to be clear, I did not intend this to be a blocker for merging. We will try to merge this PR here today before the new breaking AA release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remember adding doctests for a show method and someone complained.

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't add a doctest just for the show method; but usually there are other doctests which create objects that are printed by that show method, and so we get tests as a side effect. E.g. PR #1431 adds doctest as examples in the docstring of identity_hom, compose and map_from_func.

end
end


################################################################################
#
# IdentityMap
Expand All @@ -44,14 +59,26 @@
domain(f::IdentityMap) = f.domain
codomain(f::IdentityMap) = f.domain


function show(io::IO, ::MIME"text/plain", M::IdentityMap)
io = pretty(io)
println("Identity map")
print(io, Indent(), "of ", Lowercase())
show(io, MIME("text/plain"), domain(M))
print(io, Dedent())

Check warning on line 68 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L63-L68 

Added lines #L63 - L68 were not covered by tests
end

function show(io::IO, M::IdentityMap)
println(io, "Identity map with")
println(io, "")
println(io, "Domain:")
println(io, "=======")
print(io, domain(M))
if get(io, :supercompact, false)

Check warning on line 72 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L72 

Added line #L72 was not covered by tests
# no nested printing
print(io, "Identity map")

Check warning on line 74 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L74 

Added line #L74 was not covered by tests
else
io = pretty(io)
print(io, "Identity map of ", Lowercase(), domain(M))

Check warning on line 77 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L76-L77 

Added lines #L76 - L77 were not covered by tests
end
end


function compose(f::AbstractAlgebra.Map(AbstractAlgebra.IdentityMap){D, D}, g::AbstractAlgebra.Map{D, C}) where {D, C}
check_composable(f, g)
return g
Expand Down Expand Up @@ -84,16 +111,26 @@
return image_fn(f)(a)::elem_type(C)
end

function show(io::IO, M::FunctionalMap)
println(io, "Map with the following data")
println(io, "")
println(io, "Domain:")
println(io, "=======")
println(io, domain(M))
println(io, "")
println(io, "Codomain:")
println(io, "========")
print(io, codomain(M))


function Base.show(io::IO, ::MIME"text/plain", M::FunctionalMap)
io = pretty(io)
println(io, "Map defined by a Julia function")
println(io, Indent(), "from ", Lowercase(), domain(M))
print(io, "to ", Lowercase(), codomain(M), Dedent())
end

function Base.show(io::IO, M::AbstractAlgebra.Map)
if get(io, :supercompact, false)
# no nested printing
print(io, "Map")

Check warning on line 126 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L126 

Added line #L126 was not covered by tests
else
# nested printing allowed, preferably supercompact
io = pretty(io)
print(io, "Map: ")
print(IOContext(io, :supercompact => true), Lowercase(), domain(M), " -> ")
print(IOContext(io, :supercompact => true), Lowercase(), codomain(M))
end
end

################################################################################
Expand Down Expand Up @@ -140,23 +177,31 @@
return image_fn(f)(a)::elem_type(C)
end

function show_short(io::IO, M::AbstractAlgebra.Map)
println(IOContext(io, :compact => true), domain(M), " -> ", codomain(M))
end

function show_short(io::IO, M::FunctionalCompositeMap)
show_short(io, M.map1)
println(io, "then")
print(io, M.map2)
function show(io::IO, ::MIME"text/plain", M::FunctionalCompositeMap)
io = pretty(io)
println(io, "Functional composite map")
println(io, Indent(), "first ", Lowercase(), map1(M))
h = map2(M)
while h isa FunctionalCompositeMap
println(io, "next ", Lowercase(), h)
h = map2(h)
end
print(io, "next ", Lowercase(), h, Dedent())

Check warning on line 189 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L180-L189 

Added lines #L180 - L189 were not covered by tests
end

function show(io::IO, M::FunctionalCompositeMap)
println(io, "Composite map consisting of the following")
println(io, "")
show_short(io, M.map1)
println(io, "then")
show_short(io, M.map2)
if get(io, :supercompact, false)

Check warning on line 193 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L193 

Added line #L193 was not covered by tests
# no nested printing
print(io, "Composite map")

Check warning on line 195 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L195 

Added line #L195 was not covered by tests
else
io = pretty(io)
print(io, "Map: ", Lowercase(), domain(M))
h = map2(M)
while h isa FunctionalCompositeMap
print(io, " -> ", Lowercase(), domain(h))
h = map2(h)
end
print(io, " -> ", Lowercase(), codomain(M))

Check warning on line 204 in src/generic/Map.jl

View check run for this annotation

Codecov / codecov/patch

src/generic/Map.jl#L197-L204 

Added lines #L197 - L204 were not covered by tests
end
end



31 changes: 11 additions & 20 deletions src/generic/MapWithInverse.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,12 @@ section_map(f::MapWithSection) = f.section

(f::MapWithSection{D, C})(a) where {D, C} = (f.map)(a)::elem_type(C)

function show(io::IO, M::MapWithSection)
println(io, "Map with section with the following data")
println(io, "")
println(io, "Domain:")
println(io, "=======")
println(io, domain(M))
println(io, "")
println(io, "Codomain:")
println(io, "========")
print(io, codomain(M))

function Base.show(io::IO, ::MIME"text/plain", M::MapWithSection)
io = pretty(io)
println(io, "Map with section")
println(io, Indent(), "from ", Lowercase(), domain(M))
print(io, "to ", Lowercase(), codomain(M), Dedent())
end

function compose(f::MapWithSection{U, C}, g::MapWithSection{D, U}) where {D, U, C}
Expand Down Expand Up @@ -65,16 +61,11 @@ retraction_map(f::MapCache) = retraction_map(f.map)

(f::MapWithRetraction{D, C})(a) where {D, C} = (f.map)(a)::elem_type(C)

function show(io::IO, M::MapWithRetraction)
println(io, "Map with retraction with the following data")
println(io, "")
println(io, "Domain:")
println(io, "=======")
println(io, domain(M))
println(io, "")
println(io, "Codomain:")
println(io, "========")
print(io, codomain(M))
function Base.show(io::IO, ::MIME"text/plain", M::MapWithRetraction)
io = pretty(io)
println(io, "Map with retraction")
println(io, Indent(), "from ", Lowercase(), domain(M))
print(io, "to ", Lowercase(), codomain(M), Dedent())
end

function compose(f::MapWithRetraction{U, C}, g::MapWithRetraction{D, U}) where {D, U, C}
Expand Down
Loading