pretty printing for maps #1424
pretty printing for maps #1424
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1424 +/- ##
==========================================
- Coverage 84.55% 84.46% -0.10%
==========================================
Files 108 109 +1
Lines 29209 29365 +156
==========================================
+ Hits 24697 24802 +105
- Misses 4512 4563 +51
... and 28 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
simonbrandhorst
changed the title
| print(io, " -> ", Lowercase(), domain(h)) | ||
| h = map2(h) | ||
| end | ||
| print(io, " -> ", Lowercase(), codomain(M)) |
There was a problem hiding this comment.
Seem this function is not tested at all?
There was a problem hiding this comment.
Maybe. Our coverage of show methods isn't great. But then there are various opinions on whether/how these should be tested.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
I remember adding doctests for a show method and someone complained.
There was a problem hiding this comment.
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.
Co-authored-by: Max Horn <max@quendi.de>
Co-authored-by: Max Horn <max@quendi.de>
src/generic/Map.jl
Outdated
| else | ||
| io = pretty(io) | ||
| print(io, "Map: ", Lowercase(), domain(M)) | ||
| h = map2(m) |
There was a problem hiding this comment.
Tests are good, even for "obvious" code, as they uncover bugs like this one:
| h = map2(m) | |
| h = map2(M) |
I'll push a fix and adjusted doctests in a moment. Since PR #1431 seems to have stalled :-( I'd rather get this PR here ready ASAP.
|
Let me know if I should do anything. |
* Bump compats * Remove functions moved to Nemo (Nemocas/Nemo.jl#1519) * Fix `ZZ` printing (Nemocas/Nemo.jl#1506) * Fix `identity_map` docstrings (Nemocas/AbstractAlgebra.jl#1431) * Fix `PosInf` docstring (Nemocas/Nemo.jl#1528) * Adapt doctests to new printing for maps Nemocas/AbstractAlgebra.jl#1424
* Bump compats * Remove functions moved to Nemo (Nemocas/Nemo.jl#1519) * Fix `ZZ` printing (Nemocas/Nemo.jl#1506) * Fix `identity_map` docstrings (Nemocas/AbstractAlgebra.jl#1431) * Fix `PosInf` docstring (Nemocas/Nemo.jl#1528) * Adapt doctests to new printing for maps Nemocas/AbstractAlgebra.jl#1424
No description provided.