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

remove uses of importall Base in Base #13526

Closed
JeffBezanson opened this issue Oct 10, 2015 · 11 comments
Closed

remove uses of importall Base in Base #13526

JeffBezanson opened this issue Oct 10, 2015 · 11 comments
Assignees

Comments

@JeffBezanson
Copy link
Member

A few submodules (SparseMatrix, LinAlg, DataFmt) use this, and it's not good practice. One concrete problem is that some of Base's exported bindings are not resolved yet (since these are submodules of Base), so you can't really import them anyway. We might want to give a warning for that case.

@andreasnoack andreasnoack self-assigned this Oct 10, 2015
@simonster
Copy link
Member

Once we do this maybe we can just deprecate importall entirely? It should never be used in published code and probably just leads to confusion. The functionality can be provided by a macro (perhaps in a package) even if the keyword is deprecated, but I think having fewer ways to import packages will make things clearer in general (#8000).

@johnmyleswhite
Copy link
Member

+1 to deprecating importall

@IainNZ
Copy link
Member

IainNZ commented Oct 10, 2015

It should never be used in published code

Why not?

JuMP, for example, exports many names, and doesn't export many others. Extensions of JuMP build on both exported and unexported, so I currently use an importall to grab the exported ones the import the remaining ones manually. Is the idea to force people to just manually import everything, not just the non-exported names?

@IainNZ
Copy link
Member

IainNZ commented Oct 10, 2015

Having said that. I wouldn't mind getting rid of it to eliminate yet another way of working with modules...

@IainNZ
Copy link
Member

IainNZ commented Oct 10, 2015

@lostanlen
Copy link

In his personal style guidelines (Dec 2013), John Myles White advises against the use of importall when extending packages — see item 33. So this issue would clarify the "Julia often gives you more freedom than you should use" situation.

@IainNZ there are 63 pages of it. What are you specifically finding interesting ?

@simonster
Copy link
Member

The reason not to use importall is that if your code defined f(x) = y but you importall X, if X later adds f(x::Int) = z then your code will no longer do the right thing if it ever calls f with an Int. If you use importall with a submodule, or if you maintain both X and the package that's importing it, this isn't a concern, but having importall in Base is inviting it to be abused. As the search shows, having importall encourages people who don't know better to importall Base or importall Gadfly, which is never really right.

@KristofferC
Copy link
Member

@lostanlen I think the interesting part is that there are 63 pages.

@IainNZ
Copy link
Member

IainNZ commented Oct 10, 2015

@lostanlen that the first few pages clearly show it being used inappropriately or in a non-critical way, as @simonster points out. Additionally, that its been used quite a few times, i.e. this isn't a hypothetical discussion.

@simonster thats a pretty good reason to ditch it.

So 👍 to the original topic of this issue, and 👍 to the deprecation.

@tkelman
Copy link
Contributor

tkelman commented Oct 11, 2015

There are some high profile packages that are abusing this right now, e.g. bindeps as one example. Do we have an automated way of tracking which functions in a module were imported and extended fron elsewhere?

@JeffBezanson
Copy link
Member Author

I think importall can be useful for small modules as in importall Base.Operators (though maybe that case can be replaced by interfaces?). importall Base specifically is especially bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants