Import styles

[Sean Hammond]

Things I don't like about the `from package import module` that we're using everywhere:

1. Makes it hard to grep the codebase for all uses of a module. Can't just grep for "foo.bar.baz", now it might be from foo.bar import baz, from .bar import baz, from . import baz, from foo.bar import gar, dar, baz, jaz, etc.

2. Increases "travel" when trying to read code. I just see `baz`, I have to go up to the top of the file to see that that is in fact `foo.bar.baz`. Multiply that across everything we import in every file, makes code much harder to understand imo.

3. Pollutes namespaces, a module with lots of `from X import Y` has a high chance of some function, local variable, etc name clashing with something imported.

4. Understanding the meaning of an import statement requires taking into account the location of the file, hurts readability imo.

5. "from package.subpackage import module" is too close to the even worse "from package.module import object", encourages it. It's simpler to just say "no 'from' imports".


If I had my way we would always use absolute imports, just always:

import h.foo.bar

And always one import per line. Each module has only one canonical way to be imported. You can grep for "import foo.bar" to find all imports, or just "foo.bar" to find all uses not just imports. You can see right away in the code whenever h.foo.bar is being used because it says h.foo.bar right there.

To keep verbosity under control name packages and modules well and don't have too deep package hierarchies.

When all else fails you can be more concise by falling back on:

import h.foo.bar.baz.gaz as gaz

This still shows up in greps for "h.foo.bar.baz.gaz", but only the import shows up, not each individual use. It introduces a little bit of travel to understand what "gaz" is when you see it used inside the module, but for the benefit of not having to write h.foo.bar.baz.gaz every time. Worth it when the full module name is really long.

[Randall Leeds]

I'm going to frame my reply with these guiding principles in place:

- Importing modules is better than importing symbols from modules

from foo import bar

bar.jump()

is better than

from foo.bar import jump

jump()

For reasons of test patching and knowing whether things come from. You've touched on both of these.

- Deep module hierarchies should be for local organization only.

It's totally fine to have whatever subpackages you like, inside your module. However, if there's things that are expected to be public, they should be "lifted" at each level, and exposed in `__all__`. In this way, other code can import these symbols without reaching deeply into the package and its subpackages / submodules and within the context of a package refactors are free to move things around without breaking the package's API.

- Following from the above: deep imports are bad.

If we obey the above, we should never have something like `from foo.bar.baz import booz` because `baz` would export `booz` and `bar` would export `baz` if there's any reason to believe `booz` is public API for the whole package. Therefore, we would expect to only ever see `from foo import booz`.

Let's jump in, then!

On Fri, Mar 20, 2015 at 9:21 AM, Sean Hammond <se...@hypothes.is> wrote:

Things I don't like about the `from package import module` that we're using everywhere:

1. Makes it hard to grep the codebase for all uses of a module. Can't just grep for "foo.bar.baz", now it might be from foo.bar import baz, from .bar import baz, from . import baz, from foo.bar import gar, dar, baz, jaz, etc.

If we follow the guidelines I laid out above, this is still somewhat true. However, the times when this is necessary should hopefully be few. More often, perhaps, you need to find all the places where external code uses some module -- in which case it's importing it in a predictable way as "public" interface of the module with only one level of import -- or you need to look within some module for local usage where there's not likely to be as much name clash.

 

2. Increases "travel" when trying to read code. I just see `baz`, I have to go up to the top of the file to see that that is in fact `foo.bar.baz`. Multiply that across everything we import in every file, makes code much harder to understand imo.

If the module names are descriptive that shouldn't be an issue and decreases significantly with familiarity with the project. I'm not particularly concerned about it.
 

3. Pollutes namespaces, a module with lots of `from X import Y` has a high chance of some function, local variable, etc name clashing with something imported.

It's happened maybe once within this project that I can remember. It was temporary and a refactoring got rid of it. Python has "as ___" as a solution, too.
 

4. Understanding the meaning of an import statement requires taking into account the location of the file, hurts readability imo.

I get this. I think context dependence is not a bad thing, though. Humans are great at it in natural languages. The extent to which this is a problem might have more to do with module cohesion and isolation.
 

5. "from package.subpackage import module" is too close to the even worse "from package.module import object", encourages it. It's simpler to just say "no 'from' imports".

I think if we follow the guidelines I put at the top that this shouldn't happen. Obviously, we haven't always done this well. I would love to change that.
 

If I had my way we would always use absolute imports, just always:

import h.foo.bar

And always one import per line. Each module has only one canonical way to be imported. You can grep for "import foo.bar" to find all imports, or just "foo.bar" to find all uses not just imports. You can see right away in the code whenever h.foo.bar is being used because it says h.foo.bar right there.

To keep verbosity under control name packages and modules well and don't have too deep package hierarchies.

I think the deep hierarchies are fine, they should just be constrained to local relevance. Never export your deep hierarchy and no one should have to import it that way. Use it to organize a package, though.

I think we're actually broadly in agreement. You want to use the full import path and avoid the hierarchies, whereas I think the hierarchy is fine for internal use but the depth should be hidden by sensible exports.

The only point of departure, then, is:

from foo import bar

bar.jump()

vs

import foo.bar

foo.bar.jump()

I think the number of times the latter is likely to be clearer than the former is small enough that the verbosity is a bigger cost. I'm happy to entertain the opposition view, but I'm not convinced yet.

Finally, +1 for one import per line. I've violated this constantly and Nick brought it to my attention that it makes diffs clearer if you do one per line, so let's do that.

[Randall Leeds]

Follow-up just to add that using "import x.y" is not something I really oppose. I just think that there are many cases where "y" is descriptive enough, given the context, that "from x import y" results in code that is as readable and more pleasant.

[Randall Leeds]

Ugh, hitting send too soon...

The below stated, *if* the submodule isn't clearly named such that "x.y" is significantly more descriptive then absolutely do so.

[Nick Stenning]

Hi Sean,

I think you raise some great points. I'm all ears for ways we can make our code more readable, so thank you for sending this.

TL;DR: For the reasons below, and taking account of Randall's email which has arrived since I started writing this, I suggest we agree to eliminate relative imports, but stick with using from foo.bar import baz as an aid to readability.

1. Makes it hard to grep the codebase for all uses of a module.

I don't think it makes it hard, although it might make it slightly harder. Any "foo.py" anywhere can only be imported by a line that includes the string "foo", so grepping for that will always find all imports of that module. Sure, it may also find some other stuff, but in my experience this has never really been a problem. Even

grep -r foo . | grep import

almost always reduces the number of false positives to a level so low that it's not a problem.

2. Increases "travel" when trying to read code. I just see baz, I have to go up to the top of the file...

I think you have a fair point here, but this is heavily context sensitive. A common pattern is having a set of "adapter" modules which are all supposed to conform to the same interface, say,

foo.renderers.csv
foo.renderers.json
foo.renderers.xml

each of which is expected to contain a render function. In such a case, if people are in the habit of doing

from foo.renderers.csv import render
(... many lines omitted...)
render(data)

then I'd agree that it's confusing. But that confusion can be just as easily reduced by doing

from foo.renderers import csv
(...)
csv.render(data)

So I'd agree insofar as not all uses of this kind of import are good for code readability, but they're not all bad, either.

3. Pollutes namespaces, a module with lots of from X import Y has a high chance of some function, local variable, etc name clashing with something imported.

In practice I don't think I've run into this frequently. If your module is importing All The Things, then chances are it's a bootstrap or other glue code module, and probably shouldn't be defining lots of its own names. If your module is defining lots of its own names and also importing All The Things, that sounds to me like bad factoring, and the fact that you end up running into namespace collisions is probably a good thing in disguise -- it serves as a hint that your module is doing too much.

Either way, most editors/linting tools will flag this kind of error immediately, and if the worst comes to the worst, imports can be renamed.

4. Understanding the meaning of an import statement requires taking into account the location of the file, hurts readability imo.

This only applies to relative imports, and not to absolute from foo import bar imports.

And I think it's probably your strongest argument. When I'm jumping around source files I'm not always immediately aware of where I am in the package hierarchy.

That said, there is one consideration which to my mind argues in favour of relative imports: modules which are orthogonal to one another shouldn't frequently be importing code from levels above them in the package hierarchy -- it implies, in fact, that they're not orthogonal. Orthogonal modules would usually only import layers below them in the package hierarchy, and in that sense lots of from .. or from ... can serve as a code smell.

5. "from package.subpackage import module" is too close to the even worse "from package.module import object", encourages it. It's simpler to just say "no 'from' imports".

A fair point, but this isn't a dichotomy. We can say importing module members is not a good idea without also banning from package import module.

That said, I would happily agree that from package.module import foo, bar, baz, ... is a code smell, and should usually be replace with from package import module.

---

So, in summary, I think I accept that maybe relative imports aren't a great idea, but I'm afraid I do think that using exclusively the import foo.bar.baz style actually hurts readability, if anything. Which is more readable:

import foo.db
import foo.auth.permits
import foo.renderers.csv

def render(request, data):
    conn = foo.db.connect()
    if foo.auth.permits(request.user, 'thingamijig'):
         foo.renderers.csv.render(data)

or

from foo import db
from foo import auth
from foo.renderers import csv

def render(request, data):
    conn = db.connect()
    if auth.permits(request.user, 'thingamijig'):
         csv.render(data)

I certainly know which I prefer!

---

In order to inform the discussion with the practices of other Python projects, I checked out a couple of the big ones. For each, I've counted how many of each style of import they use within the project (i.e. not counting external dependency imports):

• absolute: import foo.bar.baz
• absolute: from foo.bar import baz
• relative: from . import baz

Counting with:

for pkg in boto django jinja2 requests; do
    printf "%10s import %4d\n" $pkg $(ag --python '^import '$pkg $pkg | wc -l)
    printf "%10s from   %4d\n" $pkg $(ag --python '^from '$pkg $pkg | wc -l)
    printf "%10s from . %4d\n" $pkg $(ag --python '^from \.' $pkg | wc -l)
done

The results:

    boto import  215
    boto from   1304
    boto from .    0
  django import   23
  django from   5156
  django from .  569
  jinja2 import    2
  jinja2 from    148
  jinja2 from .    0
requests import    2
requests from     11
requests from .  213

So:

• none of these projects really uses the import foo.bar.baz form
• django is a big project that has clearly standardised on the absolute from django.foo import bar form
• requests uses relative imports

---

See the top for my conclusion. Sorry for the mammoth email!

-N

[Randall Leeds]

Analysis FTW.

FWIW I still like the relative form. I wonder if Django predates the PEP for explicit relative imports. I'll concede this, though, so absolute it is.

Should we also add "from future import absolute_imports" or whatever, too? I think it's probably a good practice to guarantee not to accidentally import relative modules.

--
You received this message because you are subscribed to the Google Groups "Hypothesis Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to dev+uns...@list.hypothes.is.
To post to this group, send email to d...@list.hypothes.is.
To view this discussion on the web visit https://groups.google.com/a/list.hypothes.is/d/msgid/dev/B73ADBF2-4752-4D70-8D24-7661D8081D14%40hypothes.is.

[Nick Stenning]

On 20 Mar 2015, at 20:28, Randall Leeds wrote:

> Should we also add "from future import absolute_imports" or whatever, too?
> I think it's probably a good practice to guarantee not to accidentally
> import relative modules.

https://github.com/hypothesis/h/pull/2063

:)

-N

[Sean Hammond]

On 20/03/15 19:10, Nick Stenning wrote:

TL;DR: For the reasons below, and taking account of Randall's email which has arrived since I started writing this, I suggest we agree to eliminate relative imports, but stick with using from foo.bar import baz as an aid to readability.

Can't we just use "import foo.bar.baz as baz" then?

This is more consistent and easier to grep for - both import foo.bar.baz and import foo.bar.baz as baz start with the same prefix.

You can't import objects this way - "import h.api.models.Annotation" will fail whereas "from h.api.models import Annotation", by avoiding the "from" style we also conveniently avoid importing objects directly. This makes our coding standards simpler: "don't use  from", rather than "do use from but only to import modules not objects".

So in your which is more readable example:

import foo.db as db
import foo.auth as auth
import foo.renderers.csv as csv

def render(request, data):
    conn = db.connect()
    if auth.permits(request.user, 'thingamijig'):
         csv.render(data)

I see that the "from" style seems to be very popular in successful projects but I don't really get what it gives us other than disadvantages, over "import ..." and "import ... as ...".

I don't want to debate this forever so I'm also happy to go with Nick's suggestion as well.

[Randall Leeds]

It's readability, IMO. No need to repeat a namespace all over the place, when that's not necessarily very descriptive of what it's doing on any given line, whereas the deepest module is likely to be the meat/purpose of the line.

<shrug> that's how I see it.

--

You received this message because you are subscribed to the Google Groups "Hypothesis Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to dev+uns...@list.hypothes.is.
To post to this group, send email to d...@list.hypothes.is.

To view this discussion on the web visit https://groups.google.com/a/list.hypothes.is/d/msgid/dev/550FEEAB.6060604%40hypothes.is.

[Nick Stenning]

On 23 Mar 2015, at 11:44, Sean Hammond wrote:

On 20/03/15 19:10, Nick Stenning wrote:

TL;DR: For the reasons below, and taking account of Randall's email
which has arrived since I started writing this, I suggest we agree to
eliminate relative imports, but stick with using |from foo.bar import
baz| as an aid to readability.

Can't we just use "import foo.bar.baz as baz" then?

I think for now we stick with the common pattern: preferring from absolute.package.path import module

Very happy to revisit this some time in the future, but for now I think sticking to "least surprising for other Python programmers" is probably the right thing to do here.

-N