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.

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

RFC: make default module-prefix showing independent of the state of Main(e) #29466

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JeffBezanson
Copy link
Member

Currently, whether module prefixes are included in type name representations depends on what you have using'd in Main. This is not good. For example:

julia> module Foo
       using Sockets
       f() = repr(IPv4)
       end

julia> Foo.f()
"Sockets.IPv4"

julia> using Sockets

julia> Foo.f()
"IPv4"

In other words, behavior of code in some random place depends on what you happen to have loaded in your session. The idea behind this (more concise object representations at the REPL) makes sense, but I believe it should be implemented by setting the :module IOContext variable instead of by making Main the default.

With this change, both outputs above are "Sockets.IPv4", but the type is still shown as just IPv4 in the REPL.

@JeffBezanson JeffBezanson added display and printing Aesthetics and correctness of printed representations of objects. minor change Marginal behavior change acceptable for a minor release labels Oct 2, 2018
@@ -446,6 +446,9 @@ function isvisible(sym::Symbol, parent::Module, from::Module)
isdefined(from, sym) # if we're going to return true, force binding resolution
end

module HasOnlyDefaultImports
Copy link
Member

Choose a reason for hiding this comment

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

Clever!

@nalimilan
Copy link
Member

Makes sense. Can you confirm it won't affect e.g. the output of doctests in Documenter manuals?

@JeffBezanson
Copy link
Member Author

Documenter will have to be updated to pass the :module property.

@StefanKarpinski
Copy link
Member

This change had the advantage of making long spellings of names that are available at the REPL much shorter, which this PR does away with. Have you actually encountered people being really confused by this or is th is more of a theoretical concern? I have not personally found that people are confused by the fact that what’s available in the current session affects how things are displayed in that session.

@JeffBezanson
Copy link
Member Author

This is not supposed to affect how things are displayed --- that can be retained by having the REPL set the :module property on its output stream, which is what that property is for AFAIK. Read the example in the OP carefully. If, say, some package code tries to stringify something, what it gets depends on the state of the user environment.

@timholy
Copy link
Member

timholy commented Oct 2, 2018

I agree with this principle. Seems likely to be breaking, though; perhaps test ImageCore would be worth a spin?

@JeffBezanson
Copy link
Member Author

Yes, this will probably break some package tests. One major cause I've found so far is calls to summary(x) that need to be summary(io, x). There are a bunch left over since summary used to be 1 argument only.

@StefanKarpinski
Copy link
Member

Right, I see what you're getting at and wholeheartedly agree that this printing shouldn't change based on what's imported into an unrelated module. I think this can be fairly legitimately argued to be a bug fix, albeit one which may cause some breakage. We'll have to be sure the breakage is fixed.

@StefanKarpinski StefanKarpinski added this to the 1.1 milestone Oct 2, 2018
@JeffBezanson
Copy link
Member Author

Next major hazard: print(io, "T = $T") and print(io, "T = ", T) are not equivalent, since in the first case converting T to a string cannot see the context from io (of course this has always been the case, but this change exposes it more). I swear I did not intend this to be about string interpolation :) Anyway, those forms probably appear equivalent to many so it's a bit of a gotcha. I'm not sure what if anything to do about it.

@StefanKarpinski
Copy link
Member

I swear I did not intend this to be about string interpolation :)

Likely story

@JeffBezanson JeffBezanson added the triage This should be discussed on a triage call label Oct 3, 2018
@StefanKarpinski
Copy link
Member

Triage notes that triage two weeks ago talked about this extensively and then wrote nothing down. Way to go triage from two weeks ago!

@vtjnash vtjnash modified the milestones: 1.x, 1.8 May 27, 2021
@vtjnash vtjnash removed the triage This should be discussed on a triage call label May 27, 2021
@jonas-schulze
Copy link
Contributor

Could https://discourse.julialang.org/t/making-code-available-to-workers/71462 be related to this? I mean, would something like this also make module.jl work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display and printing Aesthetics and correctness of printed representations of objects. minor change Marginal behavior change acceptable for a minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants