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

Module import semantics #8013

Open
dom96 opened this Issue Jun 11, 2018 · 25 comments

Comments

Projects
None yet
7 participants
@dom96
Member

dom96 commented Jun 11, 2018

One of the number 1 complaints about Nim is the semantics of import module: the fact that all identifiers can be referred to without an explicit namespace.

I keep seeing this again and again on HN/Reddit. I'd love to give developers who view this as a problem a chance to suggest alternatives. But my main reason for creating this issue is to keep track of this alternative proposal on the forum: https://forum.nim-lang.org/t/3783.

@dom96 dom96 added the RFC label Jun 11, 2018

@zah

This comment has been minimized.

Member

zah commented Jun 11, 2018

These type of requests usually come from people who have used only dynamic languages and who are not used to having a working "Go to definition" operation in the IDE (or symbol info on mouse over).

We should educate such people instead of appealing to every request to make Nim more familiar to their way of doing things. If anything, the successful statically-typed languages feature even more implicitness with their automatically inserted this pointers in calls to base methods that may be defined in another file.

@siddhantgoel

This comment has been minimized.

siddhantgoel commented Jun 15, 2018

I would also love to see this handled in some way or the other. Having some sort of blanket that tells me that X comes from Y makes it quick & easy to visually parse and understand a piece of code. If a file is importing like 5 modules, this becomes especially important. The forum post describes exactly what the problem is.

These type of requests usually come from people who have used only dynamic languages and are not used to having a working "Go to definition" operation in the IDE (or symbol info on mouse over).

I'm not sure if it's only about dynamic languages or not having IDE support all the time. A lot of compiled statically typed languages support this (Go, for example). And you do end up reading a lot of code outside of your editor (say, on Github when you're trying to read some library your code is using).

I don't know - "explicit is better than implicit" is something I've come to really really appreciate. Doesn't matter if successful statically-typed languages support them or not. It could be that they're successful despite being implicit.

Anyway, I'd love to see this be a part of Nim in some form or the other. Thanks for the work you're putting in!

@dom96

This comment has been minimized.

Member

dom96 commented Aug 15, 2018

I'm going to mark this as accepted since @Araq has pretty much accepted it in the forum.

@dom96 dom96 added the RFC: Accepted label Aug 15, 2018

@dom96

This comment has been minimized.

Member

dom96 commented Aug 15, 2018

Quoting @Araq on the forum:

Here is my alternative proposal:

  • If we have a module M with a type T and a proc p that takes T as a first parameter, attach 'p' to 'T' much like in conventional OOP languages.
  • When you do from M import T all its operations are available too but only via the dot call syntax, obj.foo. Operators are special and also available.

This will also have further benefits for how to do symbol bindings in generics later on, if a type implements an == and hash the from x import T syntax should not lead to the hiding of these operations. Plenty of operations are affected by this, they should be more like override than like overload.

@timotheecour

This comment has been minimized.

Collaborator

timotheecour commented Aug 15, 2018

This alternative proposal doesn't work well with generics (a common case, eg algorithms.reverse), eg:
foo.nim:

type Foo=int
proc bar1(a:Foo)=discard
proc bar2[T](a:T)=discard

main.nim

from foo import Foo
var myfoo: Foo
myfoo.bar1 #ok
myfoo.bar2 #not ok, would have to explicitly import `bar2` or not use UFCS and use `foo.bar2(myfoo)`

I propose the following which allows to keep module qualified names in UFCS chains:

let a=getSomePath[0].cleanup.algorithms.reverse.furtherProcessing # doesn’t work, obviously
let a=algorithms.reverse(getSomePath[0].cleanup).furtherProcessing # breaks UFCS; gets worse if more symbols need module qualification, eg:
# mod.bar(mod.bar2(mod.bar3(arg)))

let a=getSomePath[0].cleanup.algorithms::reverse.furtherProcessing # my proposal 1 (or any simple symbol if :: is not good for some reasons, although there is precedent for it in C++ and rust)
let a=getSomePath[0].cleanup.(algorithms.reverse).furtherProcessing # alternative proposal 2
@drslump

This comment has been minimized.

Contributor

drslump commented Aug 15, 2018

When you do from M import T all its operations are available too but only via the dot call syntax, obj.foo

Could anyone explain why have the distinction of only allowing obj.foo? Otherwise the feature looks great to me, it makes code reviewing much more tractable since it's usually easy enough to figure out a type from its surroundings.

This alternative proposal doesn't work well with generics

At least for your example I think it does, I can specify that I want to import a type Foo and also a generic function bar2.

@timotheecour

This comment has been minimized.

Collaborator

timotheecour commented Aug 15, 2018

At least for your example I think it does, I can specify that I want to import a type Foo and also a generic function bar2.

for things like std/algorithms or any module that uses generics, you'll end up importing most procs, which doesn't address the concern this issue was trying to solve (see https://forum.nim-lang.org/t/3783#23584)

Again, here is how it would look like using my proposed :::

let a=getSomePath[0].cleanup.algorithms::reverse.algorithms::fill(0)
# what that expression means is:
let a=algorithms.fill(algorithms.reverse(getSomePath[0].cleanup), 0)# (hard to read and visually parse)

Note: @Araq suggested using |> in IRC but I don't know what he means; how would above expression look like with |> ?
From IRC (bridge bot) @FromIRC 14:31

@dom96

This comment has been minimized.

Member

dom96 commented Aug 15, 2018

getSomePath[0] |> cleanup |> algorithms.reverse |> algorithms.fill |> 0
@timotheecour

This comment has been minimized.

Collaborator

timotheecour commented Aug 15, 2018

thanks for clarifying. That objectively looks more foreign (to me at least) than proposed ::
let's add a ",":

let a=getSomePath[0].cleanup.algorithms::reverse.algorithms::fill(0, "bar").finalize
# not clear how to parse that (to me):
getSomePath[0] |> cleanup |> algorithms.reverse |> algorithms.fill |> 0, "bar" |> finalize
@Araq

This comment has been minimized.

Member

Araq commented Aug 15, 2018

That proposal of mine deals with a different, but related problem and still nobody ever made a convincing argument! So how does Go deal with this problem? Last time I checked it uses x.foo everywhere instead of module.foo(x) too...

EDIT: Er .... well ok I did propose a more useful from M import T feature, but still... what's the point, without IDE support I have to guess where to find a name. And that's true for Python, Go, C#, C++, C, Lua, ...

@Araq Araq removed the RFC: Accepted label Aug 15, 2018

@drslump

This comment has been minimized.

Contributor

drslump commented Aug 15, 2018

Ooops! and here I was thinking what a stroke of genius that was to make explicit imports ergonomic 😄

Thanks for clarifying!

@treeform

This comment has been minimized.

Contributor

treeform commented Aug 16, 2018

I like the way nim does imports. Please don't change.

@drslump

This comment has been minimized.

Contributor

drslump commented Aug 16, 2018

I did propose a more useful from M import T feature, but still... what's the point, without IDE support I have to guess where to find a name

The point is simplify reading code bases that aren't very familiar to the reader, and that includes:

  • code reviewing on a large team
  • skimming over a library's source code on github
  • minor: make snippets more explicit for casual sharing (email, docs, forum, stackoverflow, ...)

To me the code reviewing one is a killer feature, it probably depends on how a team works and I'm sure it varies a lot across the industry, but for anyone using a "microservices" approach on a large team you end up having to review a mix of languages/libraries/domains and every bit that helps with that counts. Improving from m import T makes it a viable alternative that can be opt-in by those that value that explicitness while not affecting those that don't.

@Araq

This comment has been minimized.

Member

Araq commented Aug 16, 2018

Yes, you don't have to repeat these claims. You have to back them up with science. So how do I figure out where imshow is declared. Context: https://github.com/matplotlib/matplotlib/blob/master/examples/color/colormap_reference.py#L57

Must be simple, after all, Python uses these superior import rules you're talking about.

@drslump

This comment has been minimized.

Contributor

drslump commented Aug 16, 2018

Sorry, I'll try to refrain myself from repeating stuff. No science backed stuff unfortunately, just my personal experience, which amounts to nothing, and that's why I didn't claim explicit imports were superior, just that they work better for some use cases.

  • imshow is a method of ax
  • ax comes from the axes collection
  • axes is obtained from the plt.subplots function
  • plt is an alias for module matplotlib.pyplot

I don't need to compile the code in my head when looking at the source, just be able to quickly navigate it visually, often times I'll find on the way some interface I know so no need to traverse the whole path all the time.

@Araq

This comment has been minimized.

Member

Araq commented Aug 16, 2018

Ok, that remains completely unconvincing, sorry. In practice I git clone the repo and search for it with the best tool at hand and are far more quickly to get reliable results than this educated guess of tracking stuff backwards where you pray no subtyping in the axes collection occurs and who knows what other implicit assumptions this tracking makes.

@zah

This comment has been minimized.

Member

zah commented Aug 16, 2018

Better long term-solution that will appear - A browser extension targeting Github overlays additional semantic information (such as hover tooltips and "Go to definition" on right click) right into the rendered code on Github.

@Araq

This comment has been minimized.

Member

Araq commented Aug 16, 2018

Here is an argument that is much more convicing to me:

from tables import CountTable

var x: CountTable[string]

x.add 4 
# Error: type mismatch
# list of 'add' candidates here but exluding Table.add!

But this error message will be improved in a different way, we won't list the overloads that don't match the first argument if there any where the first argument matches.

@Araq

This comment has been minimized.

Member

Araq commented Aug 16, 2018

And as a peace treaty we might as well add from M import T with the outlined scope injections. For me it's a pure "feeling good" feature, but we want our users to feel good. :-)

@drslump

This comment has been minimized.

Contributor

drslump commented Aug 16, 2018

that's good to hear! If someone offers to mentor me I would be super glad to work on the feature.

@Araq

This comment has been minimized.

Member

Araq commented Aug 16, 2018

Edit compiler/importer.nim to import "stuff that obviously belongs to T" for "from M import T" if it's enabled via {.experimental: "typeImports".}.

@dom96

This comment has been minimized.

Member

dom96 commented Aug 16, 2018

And as a peace treaty we might as well add from M import T with the outlined scope injections. For me it's a pure "feeling good" feature, but we want our users to feel good. :-)

That is all I ask for. This isn't about changing what import module means, it's about making the from module import Type better.

I do consider it an experiment, and I would like to hear whether it makes Python users more comfortable.

@Araq

This comment has been minimized.

Member

Araq commented Aug 16, 2018

I do consider it an experiment, and I would like to hear whether it makes Python users more comfortable.

I agree, which is why this needs to be enabled via {.experimental: "typeImports".}.

@dom96

This comment has been minimized.

Member

dom96 commented Aug 16, 2018

Yes, you don't have to repeat these claims. You have to back them up with science. So how do I figure out where imshow is declared. Context: https://github.com/matplotlib/matplotlib/blob/master/examples/color/colormap_reference.py#L57

Must be simple, after all, Python uses these superior import rules you're talking about.

Another argument:

Sure, there are cases where it's not easy to find where symbols are defined in Python. But you know the saying "Don't make perfect the enemy of the good?" of course you do. Python might not make 100% of symbols easy to find, but it does make it easy to find the 95% of symbols in your code. To most people that's a huge benefit.

@Araq

This comment has been minimized.

Member

Araq commented Aug 16, 2018

It took me very little time to find this example, for me Python code is full of this problem, so you would need to back up that 95% number.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment