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

Support using Statistics as Stat #52784

Open
LilithHafner opened this issue Jan 6, 2024 · 11 comments · May be fixed by #52821
Open

Support using Statistics as Stat #52784

LilithHafner opened this issue Jan 6, 2024 · 11 comments · May be fixed by #52821
Labels
domain:packages Package management and loading kind:feature Indicates new feature / enhancement requests parser Language parsing and surface syntax

Comments

@LilithHafner
Copy link
Member

I propose that using Statistics as Stat loads all exported names in Statistics but does not load the name Statistics itself, instead loading the module as the name Stat.

This almost is equivalent to

using Statistics
using Statistics: Statistics as Stat

And is equivalent to

using Statistics: Statistics as Stat
eval(Expr(:using, Expr(:(:), (Expr(:., s) for s in sort!(filter!(Base.Fix1(Base.isexported, Stat), names(Stat,all=true)), by=(!=(:Statistics))))...)))
@LilithHafner LilithHafner added domain:packages Package management and loading parser Language parsing and surface syntax kind:feature Indicates new feature / enhancement requests labels Jan 6, 2024
@vtjnash
Copy link
Sponsor Member

vtjnash commented Jan 6, 2024

And is equivalent to

It seems that it should be equivalent to this, but there is a warning that seems unintended with attempting it:

julia> using Statistics: Statistics as Stat

julia> global Statistics

julia> using .Stat
WARNING: using Statistics.Statistics in module Main conflicts with an existing identifier.

@LilithHafner
Copy link
Member Author

@vtjnash, am I correct in thinking that the using syntax does not load names by explicitly enumerating the exported names of the loaded module but instead simply adds the name of the loaded module to the list of modules used by the parent, done here:

arraylist_push(&to->usings, from);

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jan 8, 2024

Apparently the history of this warning is #4715, so this might specifically need to avoid the warning there when using the .Stat module tries to bring it its own name (from the implicit export Statistics)

@LilithHafner
Copy link
Member Author

Declaring a global is also an observable property and I don't think it makes sense to declare a global named Statistics on using Statistics as Stat

julia> Statistics
ERROR: UndefVarError: `Statistics` not defined in `Main`
Suggestion: check for spelling errors or missing imports.

julia> using Statistics: Statistics as Stat

julia> global Statistics

julia> using .Stat
WARNING: using Statistics.Statistics in module Main conflicts with an existing identifier.

julia> Statistics
ERROR: UndefVarError: `Statistics` not defined in `Main`
Suggestion: add an appropriate import or assignment. This global was declared but not assigned.
Hint: a global variable of this name also exists in Statistics.

julia> names(Main)
8-element Vector{Symbol}:
 :Base
 :Core
 :InteractiveUtils
 :Main
 :Statistics

@DilumAluthge
Copy link
Member

Just to clarify, what's the current behavior of using Foo as Bar?

@mbauman
Copy link
Sponsor Member

mbauman commented Jan 8, 2024

It's not valid syntax:

julia> :(using Foo as Bar)
ERROR: ParseError:
# Error @ REPL[1]:1:8
:(using Foo as Bar)
#      └─────────┘ ── `using` with `as` renaming requires a `:` and context module
Stacktrace:
 [1] top-level scope
   @ none:1

@LilithHafner
Copy link
Member Author

The Julia object model does have the notion of "using all the exported symbols except for one". As I see it we have a few options if we want to implement this

  1. Create an explicit binding for the module name when running using Mod, using Mod as M, or using Mod.SubMod (this is already sometimes the case) and stop implicitly exporting the module name itself (technically breaking but likely a minor change)
  2. 1 but also add a special case to the Base.isexported function to make it's behavior stay the same (note: Base.isexported is not part of the public API of Julia <= 1.10)
  3. When running using Mod as M, define a binding that shadows Mod
  4. Make the object model more expressive

I'm currently investigating option 1. Open to alternative suggestions.

@fredrikekre
Copy link
Member

Seems like a pretty niche usecase where you happily accept all exported names from using the module but not the module name itself? What else does it offer over using Foo; const F = Foo? The import/using logic is already pretty complicated and something new users are often confused by, perhaps it isn't worth adding complexity? In particular not if it is breaking?

@LilithHafner
Copy link
Member Author

The point is not so much to exclude loading the module name itself, but to support the syntax at all. I think this syntax is more intuitive and readable:

using LongPackageName as LPN
using LongPackageName; const LongPackageName = LPN

I do not think that the ability to rename packages on using will confuse new users because this renaming syntax is already supported in all other locations in using/import. The use case of wanting to load a long package under a short alias is reasonable in my opinion and may help allow folks to use descriptive package names without loosing brevity in domain specific usage.

If we are agreed that we want the using LongPackageName as LPN syntax, then we kinda have to make it not load LongPackageName which, sadly, does add some complexity.

This can be implemented in a technically non-breaking way (though I think the current PR is already practically non-breaking), the tradeoff there is that to make it technically non-breaking requires a more complex mental model and/or a more complex object model.

@fredrikekre
Copy link
Member

I think this syntax is more intuitive and readable:

I disagree; using LongPackageName as LPN feels like it should do what import LongPackageName as LPN does today.

@aplavin
Copy link
Contributor

aplavin commented Jan 8, 2024

FWIW, when import ... as ... syntax was introduced, I tried using ... as ... expecting it to work as well, and was disappointed it isn't supported. Agree that the semantics are clear, as @LilithHafner proposes.

The same arguments as for import as are suitable here: yes, using ... + const ... is possible, but using as is more natural. Especially given import as already exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:packages Package management and loading kind:feature Indicates new feature / enhancement requests parser Language parsing and surface syntax
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants