Overhaul special printing macros

[lgoettgens]

Inspired by oscar-system/Oscar.jl#3288 (comment).

This PR contains the following minor changes:

• Document the behavior of the special printing macros and their helper functions.
• Introduce get_name as an alternative to find_name in downstream packages, that does the same steps as the printing to find a name (e.g. first look into attribute storage before finding a new name in the global namespace). find_name should get changed to being private once all call-sites use get_name.

One fundamental change/question is the following:
It was motivated by oscar-system/Oscar.jl#3288 (comment), where we tried to enable @show_name for multivariate polynomial rings. The issue was that rings print the name that was first assigned to them (to be correct: the lexicographic first name at the moment when the first show function of the ring was called). This led to the following problem:
In Doctest1 (inside say DocModule1) we have R, _ = polynomial_ring(QQ, 3). Later, in Doctest2 (inside say DocModule2), we have S, _ = polynomial_ring(QQ, 3). As this object gets cached, it remembered its initial name R and prints as R even though it is only known as S in the current scope (DocModule2.R is not assigned).

Furthermore, the following is unintuitive:

julia> A = some_object_with_attribute_storage

julia> show(IOContext(:compact => true), A)
A

julia> B = A;

julia> A = 0;

julia> show(IOContext(:compact => true), B)
A

I would expect this now to be printed using the only valid name of the object B.

After talking to @thofma, there seems to be 3 ways how to handle name finding:

1. Cache it the first time show is called. Current functionality, leads to the issue explained above.
2. Don't cache anything (except some user / library code calls set_name! explicitly) and instead find a name to print each time it is needed. This is currently implemented in this PR. The only issue I can think of is that printing of an object (say in variable R) changes by executing A = R, as R now has the name A (again the lexicographic first one gets chosen).
3. Cache it the first time show is called. When looking up a name from the cache, verify that the name still points to the same object. If yes, be happy and take this name. If not, find a new name and put this into the cache.

Before I spend more time on this, I would like to know which of these three is the wished functionality.

Pinging @fieker @fingolfin @thofma @joschmitt to leave their opinion on this here (as you either worked on or suffered due to this code).

[codecov]

Codecov Report

Attention: 51 lines in your changes are missing coverage. Please review.

Comparison is base (f7a5678) 87.15% compared to head (a4b503a) 87.04%.

Files	Patch %	Lines
src/PrettyPrinting.jl	17.74%	51 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1594      +/-   ##
==========================================
- Coverage   87.15%   87.04%   -0.11%     
==========================================
  Files         115      115              
  Lines       29576    29593      +17     
==========================================
- Hits        25776    25760      -16     
- Misses       3800     3833      +33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[joschmitt]

I would go for 2). Assuming that it doesn't take too much time to find out the name, I don't see a benefit from caching. Especially not, if one has to check every time if this is still the right name.

[lgoettgens]

I would go for 2). Assuming that it doesn't take too much time to find out the name, I don't see a benefit from caching. Especially not, if one has to check every time if this is still the right name.

Printing is relatively slow anyway, so this shouldn't make any notable difference.

@ fieker confirmed that 1) is a bad idea (see oscar-system/Oscar.jl#3288 (comment)).

So I will continue with 2). This PR is now ready from my POV. Everything still is non-breaking. Once this is released, I will update some things downstream that use this code (in a more or less wrong/weird way). And in the next breaking release, I would like to then do some technically breaking changes to this (that won't affect our projects due to the prior changes).

[fingolfin]

Just a quick note, as a variant 3, one could "validated caching": that is, once a name has been found, cache it, but also store that this was a name determined via searching get_current_module() . Then when it is time to use, it, check whether isdefined(get_current_module(), thatName) -- if true then use it, if false, then clear it and proceed as if no name was set.

This has two advantages:

1. it avoids some overhead,
2. more importantly to me, it enhances stability of the name if the user assigns the object to multiple variables: i.f. if you do R = makeSomeObject() and then print, the name R is used. But if you later do S = R then which name will it use from here on, S or R ?

[fingolfin]

What is really missing is IMHO a text that explains

• how these things interact (in particular: what has precedence over what when it comes to determining a name)
• how to use the macros in show functions (e.g. to place them first; in which order; possibly how they related to (supercompact), oneline and detailed printing)

That's not an objection to this PR which starts to untangle and document thing, documenting each function and macro certainly is already helpful; but it is still difficult (if it is possible at all) to get the full picture just from reading those. So we need something to tie it all together

[lgoettgens]

The precedence is in the docstring of get_name.
And examples of how to use everything are in the markdown doc page.

[fieker]

My 2 pennies:

• what is the difference between find_name and get_name?
• maybe explain what the point of extra_name is, e.g. in the case of free modules wihtout a name, when the ring has a name, it might generate R^n, or maximal order O_K of the field is named, or similar. It surrently is heavily under used.
  Shall we drop it? (different discussion)
• I have no idea if the control flow now is as before (other than also checking for supercompact) or not
• the named printing was always only going to be useful in interactive mode. It was never meant to be perfect - in particular as this is not possible
• be my guest

[lgoettgens]

Just a quick note, as a variant 3, one could "validated caching": that is, once a name has been found, cache it, but also store that this was a name determined via searching get_current_module() . Then when it is time to use, it, check whether isdefined(get_current_module(), thatName) -- if true then use it, if false, then clear it and proceed as if no name was set.

isdefined(get_current_module(), thatName) is not enough to check as the variable could be used for something else. So it would be isdefined(get_current_module(), thatName) && get_current_module().thatName === obj

This has two advantages:

1. it avoids some overhead,
2. more importantly to me, it enhances stability of the name if the user assigns the object to multiple variables: i.f. if you do R = makeSomeObject() and then print, the name R is used. But if you later do S = R then which name will it use from here on, S or R ?

One change is that the name search is no longer called for every print, but only for compact and supercompact printing (to make it more performant). And in your case, it would choose the lexicographic first valid name for the object. So in your example it would keep R. But if you did S = someObject(), and later R = S, it would then choose R from that point on.

[lgoettgens]

My 2 pennies:

• what is the difference between find_name and get_name?

find_name just searches the current namespace for a variable name. This is soon to be turned private.
get_name handles all of the precedence stuff between set_name!, find_name and extra_name. This is the function that should be called from printing code (if special printing is needed).

• maybe explain what the point of extra_name is, e.g. in the case of free modules wihtout a name, when the ring has a name, it might generate R^n, or maximal order O_K of the field is named, or similar. It currently is heavily under used.
  Shall we drop it? (different discussion)

I'll add an example.

• I have no idea if the control flow now is as before (other than also checking for supercompact) or not

Only a slight modification: We now first check for compact/supercompact properties before looking up a name. Without caching, this is more performant. Other than that, no changes

[lgoettgens]

This is now updated to @fingolfin's wish with 3.
This now works as follows (using MPoly as an example, name printing is currently not enabled for it)

julia> R, x = polynomial_ring(QQ, 4)
(Multivariate polynomial ring in 4 variables over rationals, AbstractAlgebra.Generic.MPoly{Rational{BigInt}}[x1, x2, x3, x4])

julia> show(IOContext(stdout, :compact => true), R)
R
julia> A = R
Multivariate polynomial ring in 4 variables x1, x2, x3, x4
  over rationals

julia> show(IOContext(stdout, :compact => true), R)
R
julia> show(IOContext(stdout, :compact => true), A)
R
julia> R = 4
4

julia> show(IOContext(stdout, :compact => true), A)
A

@fingolfin is this as you want it?

[fingolfin]

@lgoettgens that looks good (and yes I forget the check whether the variable content is still identical to the object -- but you guessed my intention correctly and compensated for my mistake, thank you :-) )

[lgoettgens]

@lgoettgens that looks good (and yes I forget the check whether the variable content is still identical to the object -- but you guessed my intention correctly and compensated for my mistake, thank you :-) )

I'll take this as approval.

[lgoettgens]

The succeeding matching CI verifies that oscar-system/Oscar.jl#3318 adapts Oscar for this PR to be non-breaking.