launchpad-dev team mailing list archive

[Benji York <benji.york@xxxxxxxxxxxxx>]

On Wed, Aug 4, 2010 at 5:14 PM, Robert Collins
<robert.collins@xxxxxxxxxxxxx> wrote:
>
> "Our style guide for multiline import statements differs from our general
> guideline for multiline braces, as a compromise to keep our import sections to
> a reasonable size."
>
> - I want to delete this, so that
> https://dev.launchpad.net/PythonStyleGuide#Whitespace%20and%20Wrapping
> applies to all multiline lists, not all-but-imports.

+1

I like consistency.

Another thing I like is being lazy.  The prevailing Zope import style
(especially in Zope Corp produced code) is to never use "from" imports,
import only modules, and not group imports in any way (stdlib, other
packages, our packages, etc.).

That approach has its pros and cons, but one pro I especially like is
being able to not have to think much when adding an import.  All I have
to do is add the import anywhere in the import block and then sort the
imports with my editor (in Vim I type "vip:sort<enter>", I'm sure all
other acceptable editors can do this as well).

I don't think using that style in LP would be a good idea, but it lead
me to wonder if a style with similar benefits was possible.  I came up
with this:

1) all imports have to be on one line, if a set of imports is too long,
   it is made into multiple imports.  For example:

    from very.long.string.of.modules import (very_long_name_1,
        very_long_name_2)

    Would instead be:

    from very.long.string.of.modules import very_long_name_1
    from very.long.string.of.modules import very_long_name_2

2) No grouping of "like" imports.  All the imports are in one block with
   no whitespace between them.

NOTE: These rules would require relaxing the maximum line length
    limitation for imports of very long symbol names from very long
    package paths.

Here is a before and after of a representative import block (from
lib/canonical/launchpad/webapp/tests/test_publication.py).

Before:

import logging
import sys
import unittest

from contrib.oauth import OAuthRequest, OAuthSignatureMethod_PLAINTEXT
from zope.interface import directlyProvides

from storm.database import STATE_DISCONNECTED, STATE_RECONNECT
from storm.exceptions import DisconnectionError
from storm.zope.interfaces import IZStorm

from zope.component import getUtility
from zope.error.interfaces import IErrorReportingUtility
from zope.publisher.interfaces import Retry

from canonical.config import dbconfig
from canonical.launchpad.database.emailaddress import EmailAddress
from canonical.launchpad.interfaces.lpstorm import IMasterStore
from canonical.launchpad.interfaces.oauth import (
    IOAuthConsumerSet, IOAuthSignedRequest)
from canonical.launchpad.ftests import ANONYMOUS, login
from canonical.launchpad.readonly import is_read_only
from canonical.launchpad.tests.readonly import (
    remove_read_only_file, touch_read_only_file)
import canonical.launchpad.webapp.adapter as dbadapter
from canonical.launchpad.webapp.interfaces import (
    IStoreSelector, MAIN_STORE, MASTER_FLAVOR, OAuthPermission, SLAVE_FLAVOR,
    OffsiteFormPostError, NoReferrerError)
from canonical.launchpad.webapp.publication import (
    is_browser, LaunchpadBrowserPublication, maybe_block_offsite_form_post,
    REFERRER_WHITELIST)
from canonical.launchpad.webapp.servers import (
    LaunchpadTestRequest, WebServicePublication)
from canonical.testing import DatabaseFunctionalLayer, FunctionalLayer
from lp.testing import TestCase, TestCaseWithFactory

After:

from canonical.config import dbconfig
from canonical.launchpad.database.emailaddress import EmailAddress
from canonical.launchpad.ftests import ANONYMOUS, login
from canonical.launchpad.interfaces.lpstorm import IMasterStore
from canonical.launchpad.interfaces.oauth import IOAuthConsumerSet
from canonical.launchpad.interfaces.oauth import IOAuthSignedRequest
from canonical.launchpad.readonly import is_read_only
from canonical.launchpad.tests.readonly import remove_read_only_file
from canonical.launchpad.tests.readonly import touch_read_only_file
from canonical.launchpad.webapp.interfaces import IStoreSelector, MAIN_STORE
from canonical.launchpad.webapp.interfaces import MASTER_FLAVOR, SLAVE_FLAVOR
from canonical.launchpad.webapp.interfaces import NoReferrerError
from canonical.launchpad.webapp.interfaces import OffsiteFormPostError
from canonical.launchpad.webapp.publication import LaunchpadBrowserPublication
from canonical.launchpad.webapp.publication import REFERRER_WHITELIST
from canonical.launchpad.webapp.publication import is_browser
from canonical.launchpad.webapp.publication import maybe_block_offsite_form_post
from canonical.launchpad.webapp.servers import LaunchpadTestRequest
from canonical.launchpad.webapp.servers import WebServicePublication
from canonical.testing import DatabaseFunctionalLayer, FunctionalLayer
from contrib.oauth import OAuthRequest, OAuthSignatureMethod_PLAINTEXT
from lp.testing import TestCase, TestCaseWithFactory
from storm.database import STATE_DISCONNECTED, STATE_RECONNECT
from storm.exceptions import DisconnectionError
from storm.zope.interfaces import IZStorm
from zope.component import getUtility
from zope.error.interfaces import IErrorReportingUtility
from zope.interface import directlyProvides
from zope.publisher.interfaces import Retry
import canonical.launchpad.webapp.adapter as dbadapter
import logging
import sys
import unittest

I'm slightly averse to the repeated "from foo" parts, but it sorts and
reads easily, and makes it easy to add and remove imports.

A possible variation would be to have two blocks, one for the
"import..." lines (presumably first) and one for the "from..." lines
(presumably second).
-- 
Benji York