Fixup some importing stuff.

[Pulkit07]

This PR adds from __future__ import absolute_import to files and make the imports relative.

[coveralls]

Coverage increased (+0.002%) to 99.798% when pulling 0a183fe on Pulkit07:master into 6dad14d on agronholm:master.

[agronholm]

Uh, why? It works fine as it is.

[Pulkit07]

We at Mercurial (hg) will like to use cbor for serializing and deserializing data for our state files. We cannot rely on users installing libraries using pip and we need to vendor the library ourselves. Imports were not working relatively.

For reference:
https://phab.mercurial-scm.org/D2750
https://phab.mercurial-scm.org/D2751
https://phab.mercurial-scm.org/D2752
https://phab.mercurial-scm.org/D2753

I might be doing something wrong there, feel free to point that out.

[agronholm]

This sounds like something your vendorization script needs to do. From my perspective, this PR only adds useless cruft to the codebase.

[Pulkit07]

Okay, no worries. Thanks for reviewing.

[indygreg]

This PR doesn't "add cruft." It net adds 4 non-empty lines: all of them from __future__ import absolute_import. Those lines basically say "Python 2: have your import behave like Python 3's does." In the Python projects I've been around, absolute_import is aggressively adopted because it makes Python 3 porting easier and makes the import machinery less buggy (Python 2's import attempts both a relative import and an absolute import whereas Python 3's import always imports from the root). Python 3's version is objectively better because it can avoid bugs where a local module name conflicts with a top-level module name. In the case of cbor2, import types without from __future__ import absolute_imports is ambiguous as to whether it should import cbor.types or the types top-level module that is part of the Python standard library. from __future__ import absolute_imports removes that ambiguity.

Secondly, this PR changes imports so they are relative instead of absolute. There are no regressions as a result of this change. However, there is a meaningful improvement! That improvement is that downstream consumers of Python packages using relative imports can rename packages without the package breaking. When absolute imports are used, modules MUST be available at the absolute name specified. I concede that in most Python environments, packages are at their original, intended name. However, there are Python applications (like Mercurial) that may wish to vendor Python packages in local namespaces such that there is no ambiguity between the package provided by the local application and the package that may happen to be installed on the local machine. If a standalone Python application (like Mercurial) performed an import cbor2, the cbor2 package could either come from Mercurial's distributed copy or whatever version the user had installed. And if Mercurial were not compatible with that user-installed versions, things would blow up. That's why Python applications may want to rename Python packages. Relative imports facilitate near effortless package renaming. And relative imports don't require modifying source code when importing packages. That makes downstream adoption of new package versions simpler. And that's why Python package authors should care about using relative imports.

[agronholm]

While I acknowledge that the changes are trivial, I felt that the problem was being solved from the wrong end. I've been involved in the development of core packaging tools (pip, setuptools) which have to vendor packages since, by virtue of being the very tools used to install those packages, they can't expect them to be available. Can you explain to me what makes Mercurial so special that it cannot simply declare the appropriate dependencies?

[indygreg]

I can't say if this makes Mercurial "so special," but our perspective is that of an application where Python is an implementation detail. Unlike most Python packages where the target audience is Python developers, our audience is general users - users without Python skills. Because of this, we don't want to require our users to know anything about Python or Python packaging. If they get a copy of Mercurial, it should "just work."

We can and do distribute Mercurial as a standalone application with a distributed copy of Python embedded. Here, we have total control over the environment and could install packages under their proper names. However, Mercurial is also distributed as a Python package. And in these environments we don't have full control over the Python environment. Mercurial could be running as a global Python package, as part of a virtualenv, etc. There's no way of knowing what other Python packages are present. Having Mercurial rely on non-vendored packages or packages picked up from the /outer/ Python environment is risky for a number of reasons. The most reliable behavior is to have Mercurial ensure it always imports a local, vendored copy of Python packages.

Furthermore, Mercurial supports extensions. Those extensions could have dependencies on other Python packages. There could be a scenario where an extension wishes to use a different version of a Python package from the one Mercurial is using. This can happen in environments using the system package manager to install Mercurial where Mercurial is somewhat old and an extension may wish to use a newer version of a Python package than what Mercurial is bundling. Again, having Mercurial reference a local, vendored copy of a package under a Mercurial-specific name avoids conflicts with other Python code.

I hope that provides some context.

[agronholm]

Alright, but what is absolute_import needed for? This library does not have external dependencies and thus all imports will come from the standard library anyway, even if you leave modules with the same named lying around.

[indygreg]

absolute_import isn't strictly needed. I personally find it a best practice to ensure import behaves similarly on Python 2 and 3. I also believe it is slightly faster, since import X is unambiguous and Python can avoid stat()ing paths in the local package.

[agronholm]

I just pushed a commit which switches to relative imports but I left out the absolute_imports import.

[Pulkit07]

@agronholm Thanks for the commit. If you are interested, @indygreg started a thread on python-dev related to this https://mail.python.org/pipermail/python-dev/2018-March/152446.html