Formatting of imports at the top of the file for Python3

[portante]

Below is a current example (at least it is mostly adhered to ...) method of handling the ordering of imports to satisfy PEP8 while making it easy to sort the imports alphabetically without doing it manually.

We know which imports are not from the standard library because they exist in our *-requirements.txt files.

# First bare imports from standard libary
import re

# Then from imports from standard library
from logging import Logger
from urllib.parse import urljoin

# Then bare imports from 3rd party libraries (none here)

# Then from imports from 3rd party librarires
from flask import request, jsonify
from flask.globals import current_app
from flask_restful import Resource, abort

# Then bare imports from our project (none here)

# Then from imports from our project
from pbench.server import PbenchServerConfig

[webbnh]

So, other than the textual representation, how are from imports different from bare import's? That is, why would we want them separated from each other in the file? I mean, yeah, it produces a much prettier layout, but my concern is that it facilitates having both a from import and a bare import of the same package into the same module, and, except for the rare case, that seems like something that we want to avoid.

(Although, there's a tangential discussion that we should have about when to use from and when to use a bare import...I think we over-use or misuse from, myself...but that is, as I said, a different discussion.)

My desire is that it should be easy, either as a coder or a reviewer, to know where a given import should go. If there is a question about whether to add one, it should be trivial to locate the spot in the code to see if it is already there. I'd rather not have to consult some external source to determine which block it goes in (i.e., I can easily recognize project imports, but I can't really tell standard library ones from 3rd party ones, given the ubiquity of 3rd party packages). There should be a unique location for each package so that I know where to look to see if the package has already been imported so that I can avoid adding redundant imports or so that I can find existing from imports to which I need to add a reference, etc.

I'm concerned that having as many as six blocks doesn't support that desire. And, given that each import is supposed to be self-contained, I'm having trouble figuring out why we need more than one block. (Why should project imports be segregated from external imports?...This isn't like C #include's which inject source code into the compilation stream, where ordering actually matters to the parse -- this is more like shared library symbol resolution, where the definition just has to be reachable at load-time.)

[webbnh]

Just as a data point, here's what my IDE does with the imports from pbench/agent/results.py from #2189:

import datetime
import errno
import os
import tarfile
from configparser import ConfigParser
from logging import Logger
from pathlib import Path
from typing import IO, Iterator

import requests
from werkzeug.utils import secure_filename

from pbench.agent import PbenchAgentConfig
from pbench.common.exceptions import BadMDLogFormat
from pbench.common.utils import md5sum

So, that looks like three blocks -- standard library, 3rd party, and project -- where each block contains two sections (not separated by a blank line) with bare import's before from's, and then alphabetized within each second.

I can live with that (since my IDE will do most of the work for me...). I'm not sure what the implications of having the bare import's preceding the from imports is (or if it actually matters); the distribution of blank lines is fine with me.

[portante]

Can you share what your IDE is?

[dbutenhof]

My IDE (vscode), with no manual fix-ups, does this:

from datetime import datetime
from enum import Enum
from logging import Logger
from typing import Any, AnyStr, Callable, Dict, List
from urllib.parse import urljoin

import requests
from dateutil import parser as date_parser
from dateutil import rrule
from dateutil.relativedelta import relativedelta
from flask import request
from flask_restful import Resource, abort
from pbench.server import PbenchServerConfig
from pbench.server.api.auth import Auth

So it's in two segments; it's idea of builtins separated from 3rd party, and with import before from... but it doesn't distinguish between pbench and other 3rd party. Which means that it differs from your automated sorting as well as Pete's manual sorting. It's easy enough to add another blank line before the first pbench, but that's not "automated".

I guess I'm fine with using vscode to sort and then inserting a line before the pbench. I think we should seriously consider the "over-use" of from imports. Part of this is because some modules are pretty big, and restricting the imported namespace seems like an advantage; however using the full path, e.g., dateutil.parser.parse would resolve some ambiguity in code reviews, and I really don't like having to rename imports... so that's an argument for "not what I did".

[webbnh]

I'm using JetBrains PyCharm Community (free) edition. (I didn't mean to be secretive...I just figured that, by this point, it didn't matter much because people probably have their respective approaches figured out. :-) )

I think we should seriously consider the "over-use" of from imports. Part of this is because some modules are pretty big, and restricting the imported namespace seems like an advantage; however using the full path, e.g., dateutil.parser.parse would resolve some ambiguity in code reviews, and I really don't like having to rename imports... so that's an argument for "not what I did".

Restricting the namespace on purity grounds is something that I support in principal, but I'm not inclined to put much effort into it unless there is a real space or speed argument for it. However, the improvements in readability are valuable (said as the one who goaded Dave into "not what I did"). That said though, I believe that the uses of from and as are orthogonal, so where it makes sense we can use an otherwise bare import and attach an as to it (e.g., to shorten datetime to DT) or we can do as I coerced Dave into and use from but give the result a whole new name.

[portante]

While this discussion happened a long time ago, we have since started use isort in our code and this problem is now solved. See PR #2986.