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

add import ... as ... syntax #37396

Merged
merged 2 commits into from
Oct 8, 2020
Merged

add import ... as ... syntax #37396

merged 2 commits into from
Oct 8, 2020

Conversation

JeffBezanson
Copy link
Member

closes #1255

@JeffBezanson JeffBezanson added modules parser Language parsing and surface syntax labels Sep 4, 2020
@quinnj
Copy link
Member

quinnj commented Sep 4, 2020

Now that's an old issue number!

@tkf
Copy link
Member

tkf commented Sep 4, 2020

Why not using ... as ...? I think it's reasonable to expect that

import CSV.read as rd
rd(x::Int) = x

adds a method to CSV.read. I think it is a good practice (or at least some people prefer) to use using M: f instead of import M: f to avoid accidentally overloading f and also to clarify that this function is imported just for calling it. So, isn't it better to support using CSV.read as rd instead?

Also, isn't import M.x as y redundant if import M: x as y is supported? Why not just support import M: x as y? Tools like JuliaFormatter.jl may need to implement an extra transformation if there are multiple equivalent forms. I think it might be a good idea to not introduce a redundant syntax in the first place.

@JeffBezanson
Copy link
Member Author

Ok, we can allow using as well. It's just a bit different because using X as Y makes less sense to me (since it doesn't clearly operate on a single identifier). But using M: x as y can work.

It might be ok to remove import M.x in the future, but I think as long as we have it as might as well work with it.

@tkf
Copy link
Member

tkf commented Sep 5, 2020

Thanks for the consideration. I think I can see the motivation behind import M.x as y as the syntax is more symmetry this way. I'll probably avoid it personally but it's much lower priority than having using M: x as y.

using X as Y makes less sense

I agree. I guess it has to be written as import X as Y or using X: X as Y.

@JeffBezanson
Copy link
Member Author

Added using.

@JeffBezanson JeffBezanson added the triage This should be discussed on a triage call label Sep 9, 2020
@JeffBezanson
Copy link
Member Author

Triage is ok with this. The potential for abuse is very limited, and we think this will most likely only be used on the rare occasion where it's really convenient. The downvoters are free to make their case however :)

@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Sep 10, 2020
@tkf
Copy link
Member

tkf commented Sep 11, 2020

Does it make sense to unsupport import M.x as y and import M: x as y (while keeping using M.x as y and using M: x as y)? It means that, for import, only import M as N would be supported. I'm bringing this up because it's unclear to me when you'd want to overload functions via aliases. Also, it's much harder to remove syntax than adding one. If people complain about the asymmetry between import and using, it's easy to add them later.

@jmert
Copy link
Contributor

jmert commented Sep 11, 2020

Two questions about this:

  1. Is it safe to assume there's a way to implement @compat import M.T as S in Compat.jl?
  2. Does a using of a renamed module still work if the parent module and original module have the same name? For instance, I recently wanted to name a module FileIO, but I also wanted to use the FileIO package — would this now work
    module FileIO
        import FileIO as FIO    # or `@compat import FileIO as FIO`
        using .FIO
        # ...
    end
    without there being confusion due to the fact that normally the self-name is exported (i.e. :FileIO in names(FIO) == true).

Edit: Hmm... After staring at the second question for another moment, I realized I never actually tried without renaming — I'd just assumed it wouldn't work. But on v1.5.1, I find

julia> module FileIO
           using FileIO
           const a = 1
           @show FileIO.a
           @show FileIO.load
       end
FileIO.a = 1
FileIO.load = FileIO.load
Main.FileIO

which I didn't expect. I guess something about name resolution isn't complete/accurate in my mental model.

@fredrikekre
Copy link
Member

Is it safe to assume there's a way to implement @compat import M.T as S in Compat.jl?

It already exists in https://github.com/fredrikekre/ImportMacros.jl (but with the syntax @import M.T as S -- should be trivial to update that to @compat import M.T as S and move it to Compat.)

@ararslan
Copy link
Member

Is it safe to assume there's a way to implement @compat import M.T as S in Compat.jl?

Syntactically yes but functionally not as far as I know. With this PR, the identifier T wouldn't come into scope at all, you would only have S. I believe the closest you can currently get to this (and thus how it would have to be implemented in Compat) is to do import M.T; const S = T, which does bring T into scope, or import M; const S = M.T, which brings M into scope. Whether that matters is situation-dependent, of course.

@fredrikekre
Copy link
Member

Syntactically yes but functionally not as far as I know.

You underestimate my creativity:

julia> using ImportMacros

julia> @import Example as ex

julia> Example
ERROR: UndefVarError: Example not defined

julia> ex
Example

@JeffBezanson
Copy link
Member Author

I think we should keep this as-is. It's just too surprising for import a.b as c not to work, especially since it is so commonly written in python.

@tkf
Copy link
Member

tkf commented Sep 24, 2020

There are other "surprising" API in Julia like * for string concatenation (for some people) and ** even has special error presumably targeting Python programmers. These APIs make sense once you read some documentation and often the surprise turns into appreciation. Furthermore, I think "it's familiar for Python programmers" is actually a reason to not add import a.b as c syntax because it'd mean that there is a higher chance they are using import without knowing that it's easy to accidentally add methods and modify the whole system.

@JeffBezanson
Copy link
Member Author

It just seems silly to me. ImportMacros.jl already allows it, so why would we add this syntax but then make you use that package only for one very obvious case of renaming a single identifier? Another possibility might be to disallow method extension for renamed identifiers, instead of disallowing the import syntax.

@tkf
Copy link
Member

tkf commented Sep 25, 2020

I guess it boils down to whether or not Julia wants to be opinionated about code organization. Though I guess Julia has the lispy vibe of allowing almost everything so maybe I'm arguing something nonidiomatic.

so why would we add this syntax but then make you use that package only for one very obvious case of renaming a single identifier?

Why not using a: b as c? Or do you mean to rename and extend methods? My assumption has been that allowing adding method to renamed function is not beneficial overall and mostly a footgun for new users.

Another possibility might be to disallow method extension for renamed identifiers

I think the downside is that it might introduce confusion on the difference between import and using without as.

@JeffBezanson JeffBezanson added the triage This should be discussed on a triage call label Sep 29, 2020
@JeffBezanson
Copy link
Member Author

Triage is ok with this as-is. import a.b as c is the single most obvious thing to try, so it should work. Adding methods to renamed functions is not bad enough to justify interfering with that.

Note: need to mention using in the NEWS item.

@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Oct 1, 2020
doc/src/manual/modules.md Outdated Show resolved Hide resolved
@JeffBezanson JeffBezanson force-pushed the jb/importas branch 2 times, most recently from ebfdcfa to b1399e3 Compare October 2, 2020 15:41
@JeffBezanson JeffBezanson merged commit 40fee86 into master Oct 8, 2020
@JeffBezanson JeffBezanson deleted the jb/importas branch October 8, 2020 17:16
@KristofferC
Copy link
Member

KristofferC commented Oct 8, 2020

Finnaly, I can do import LinearAlgebra as la. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules parser Language parsing and surface syntax
Projects
None yet
Development

Successfully merging this pull request may close these issues.

module and import aliasing
7 participants