Another fix for printing of scheme-related types #2864
Conversation
There was a problem hiding this comment.
I find it really confusing that the supercompact, oneline and detailed printing for these objects often use completely different taglines.
Maybe this is justified in a few cases, but in all??
I think this is partly because there is no systematic testing for this printing and so one does not even see anywhere what changes to these cause, and how the different printing options look when put next to each other.
I've created a setup for making this kind of tests a bit easier to write, you can preview it in https://github.com/Nemocas/AbstractAlgebra.jl/pull/1450/files#diff-249d4721dac939c93f2b04531e4728899bf98414e9ffeff88015a2e9516fd8b5 -- it will hopefully soon be available here (after some PR merging and releases)
| @@ -15,7 +15,7 @@ end | |||
| # one line printing | |||
| function Base.show(io::IO, X::AbsProjectiveVariety{<:Field,<:MPolyQuoRing}) | |||
| if get(io, :supercompact, false) | |||
| print(io, "Scheme") | |||
| print(io, "Projective variety") | |||
There was a problem hiding this comment.
Else case is missing this prefix and instead prints "V(...)" with "V" hardcoded that seems not good?
There was a problem hiding this comment.
Here was one of the solution to obtain explicit short printing, that could also fit well in nested printings. For a projective variety, we know the defining ideal I and it is common to use the notation V(I) to describe such objects. This is why the "V" is hardcoded, and then we print the ideal in between parentheses.
|
I will go through the comments, thank you very much for this! The differences between printing modes for a same types (with sometimes slight change of signatures) have been made on purpose, thoroughly. We can of course discuss the relevance/consistency, but words are put in a certain context and so the printings have been sometimes adapted to make them as clear as possible and avoid redundancy. I agree that for some places, viewing some of your comments, there are things to be adapted that I did not notice the first time I made all those changes. For some others I do not agree, and I guess it is important to keep those differences because we do not bring the same amount of information between detailed and non-detailed printings. The general motto was: we do not compute anything we do not know, but we put as much information as we know about the object in the (super)compact printings. In the case of |
Head branch was pushed to by a user without write access
Codecov Report
@@ Coverage Diff @@
## master #2864 +/- ##
==========================================
- Coverage 80.49% 80.48% -0.01%
==========================================
Files 456 456
Lines 65220 65214 -6
==========================================
- Hits 52501 52490 -11
- Misses 12719 12724 +5
|
| print(io, "Covered projective scheme") | ||
| print(io, "Relative projective scheme") |
There was a problem hiding this comment.
Please check this with @HechtiDerLachs -- "Covered projective scheme" has the wording of the underlying type built into it. This might be on purpose.
| println(io, "Morphism") | ||
| println(io, "Spec open morphism") |
There was a problem hiding this comment.
Not for this PR, but for later discussion:
Spec open is not a standard wording. Maybe it should be printed as a typename or with a different wording.
There was a problem hiding this comment.
Yes I agree. To be honnest I was not sure what to do with this without having very long names. At least the printing looks consistent and it carries the wording of the typename (for now)
Complement to #2856 for scheme objects.
:supercompactprintings for the scheme-related objects (which was too compact and not useful);MorphismbyHomin the compact printings of morphisms.@simonbrandhorst
(I duplicated the changes of PR-2856 for two of the types to avoid potential conflicts when one of the two PR will be merged.)