remove manipulations of sys.path (#3305)

python allows to modify the search path for modules programmatically via the `sys.path` variable (`sys.path` is initialized from the `PYTHONPATH` environment variable).
Doing so, however, can easily have unintended consequences (one of them surfaced in #3293) - in particular, when new paths are inserted at the *beginning* of `sys.path`.

AiiDA was manipulating `sys.path` in a number of places:

1. https://github.com/aiidateam/aiida-core/blob/a500d2b2801efa393ce6f567dc9893a62696b8f1/aiida/cmdline/commands/cmd_run.py#L113

1. https://github.com/aiidateam/aiida-core/blob/d750d513f7adf5126d43a230c9fb378de02f0772/aiida/sphinxext/tests/workchain_source/conf.py#L32

1. https://github.com/aiidateam/aiida-core/blob/cc94a1e0127cd9cab0a4049e69a9fa31cb82941c/utils/validate_consistency.py#L190

1. https://github.com/aiidateam/aiida-core/blob/cc94a1e0127cd9cab0a4049e69a9fa31cb82941c/aiida/common/files.py#L194

1. https://github.com/aiidateam/aiida-core/blob/91b205bcdeebc3d6f9a019c77c38ffdf6fc95321/aiida/backends/djsite/settings.py#L31

This PR removes/remedies those manipulations (except for point 4, which is part of `shutil.which` and is relevant only for `sys.platform == 'win32'`).

.pre-commit-config.yaml

@@ -146,11 +146,7 @@
         aiida/schedulers/plugins/test_sge.py|
         aiida/schedulers/plugins/test_slurm.py|
         aiida/schedulers/plugins/test_torque.py|
-        aiida/sphinxext/tests/conftest.py|
-        aiida/sphinxext/tests/test_workchain.py|
         aiida/sphinxext/tests/workchain_source/conf.py|
-        aiida/sphinxext/tests/workchain_source/demo_workchain.py|
-        aiida/sphinxext/workchain.py|
         aiida/tools/data/array/kpoints/legacy.py|
         aiida/tools/data/array/kpoints/seekpath.py|
         aiida/tools/data/__init__.py|

aiida/backends/djsite/settings.py

@@ -13,23 +13,13 @@
 from __future__ import division
 from __future__ import print_function
 
-import sys
-
-import os
 from sqlalchemy.dialects.postgresql import JSONB
 from sqlalchemy.dialects.postgresql import UUID
 
 from aiida.common import exceptions
 from aiida.common.timezone import get_current_timezone
 from aiida.manage.configuration import get_profile, settings
 
-# Assumes that parent directory of aiida is root for
-# things like templates/, SQL/ etc.  If not, change what follows...
-
-AIIDA_DIR = os.path.dirname(os.path.abspath(__file__))
-BASE_DIR = os.path.split(AIIDA_DIR)[0]
-sys.path = [BASE_DIR] + sys.path
-
 try:
     PROFILE = get_profile()
 except exceptions.MissingConfigurationError as exception:

aiida/cmdline/commands/cmd_run.py

@@ -12,7 +12,6 @@
 from __future__ import print_function
 from __future__ import absolute_import
 import contextlib
-import os
 import sys
 
 import click
@@ -109,8 +108,6 @@ def run(scriptname, varargs, group, group_name, exclude, excludesubclasses, incl
             # Must add also argv[0]
             new_argv = [scriptname] + list(varargs)
             with update_environment(new_argv=new_argv):
-                # Add local folder to sys.path
-                sys.path.insert(0, os.path.abspath(os.curdir))
                 # Compile the script for execution and pass it to exec with the globals_dict
                 exec(compile(handle.read(), scriptname, 'exec', dont_inherit=True), globals_dict)  # yapf: disable # pylint: disable=exec-used
         except SystemExit:  # pylint: disable=try-except-raise

aiida/sphinxext/tests/conftest.py

@@ -7,47 +7,54 @@
 # For further information on the license, see the LICENSE.txt file        #
 # For further information please visit http://www.aiida.net               #
 ###########################################################################
+"""Pytest fixtures for AiiDA sphinx extension tests."""
 from __future__ import division
 from __future__ import print_function
 from __future__ import absolute_import
 import os
+from os import path
 import sys
 import shutil
 import tempfile
 import subprocess
-from os.path import join, dirname
 import xml.etree.ElementTree as ET
 
 import pytest
 
+
 @pytest.fixture
 def reference_result():
+    """Return reference results (for check)."""
+
     def inner(name):
-        return join(dirname(__file__), 'reference_results', name)
+        return path.join(path.dirname(__file__), 'reference_results', name)
+
     return inner
 
+
 @pytest.fixture
 def build_dir():
+    """Create directory to build documentation."""
     # Python 2 doesn't have tempfile.TemporaryDirectory
     dirname = tempfile.mkdtemp()
-    try:
-        yield dirname
-    except Exception as e:
-        raise e
-    finally:
-        shutil.rmtree(dirname)
+    yield dirname
+    shutil.rmtree(dirname)
 
 
 @pytest.fixture
-def build_sphinx(build_dir):
+def build_sphinx(build_dir):  # pylint: disable=redefined-outer-name
+    """Returns function to run sphinx to build documentation."""
+
     def inner(source_dir, builder='xml'):
-        doctree_dir = join(build_dir, 'doctrees')
-        out_dir = join(build_dir, builder)
+        """Run sphinx to build documentation."""
+        doctree_dir = path.join(build_dir, 'doctrees')
+        out_dir = path.join(build_dir, builder)
 
-        subprocess.check_call([
-            sys.executable, '-m', 'sphinx', '-b', builder, '-d', doctree_dir,
-            source_dir, out_dir
-        ])
+        subprocess.check_call(
+            [sys.executable, '-m', 'sphinx', '-b', builder, '-d', doctree_dir, source_dir, out_dir],
+            # add demo_workchain.py to the PYTHONPATH
+            cwd=path.join(source_dir, os.pardir)
+        )
 
         return out_dir
 
@@ -56,20 +63,20 @@ def inner(source_dir, builder='xml'):
 
 @pytest.fixture
 def xml_equal():
+    """Check whether output and reference XML are identical."""
+
     def inner(test_file, reference_file):
         if not os.path.isfile(reference_file):
             shutil.copyfile(test_file, reference_file)
             raise ValueError('Reference file does not exist!')
         assert _flatten_xml(test_file) == _flatten_xml(reference_file)
+
     return inner
 
 
 def _flatten_xml(filename):
-    return [
-        (
-            el.tag,
-            {k: v for k, v in el.attrib.items() if k not in ['source']},
-            el.text
-        )
-        for el in ET.parse(filename).iter()
-    ]
+    """Flatten XML to list of tuples of tag and dictionary."""
+    return [(el.tag, {k: v
+                      for k, v in el.attrib.items()
+                      if k not in ['source']}, el.text)
+            for el in ET.parse(filename).iter()]

aiida/sphinxext/tests/workchain_source/demo_workchain.py → aiida/sphinxext/tests/demo_workchain.py

@@ -35,5 +35,5 @@ def define(cls, spec):
         spec.output('z', valid_type=Bool, help='Output of the demoworkchain.')
 
 
-class NormalClass(object):
+class NormalClass(object):  # pylint: disable=too-few-public-methods
     """This is here to check that we didn't break the regular autoclass."""

aiida/sphinxext/tests/test_workchain.py

@@ -15,12 +15,15 @@
 from os.path import join, dirname
 
 import six
-import pytest
 
 WORKCHAIN = join(dirname(__file__), 'workchain_source')
 
 
 def test_workchain_build(build_sphinx, xml_equal, reference_result):
+    """Test building sphinx documentation for WorkChain.
+
+    Builds Sphinx documentation for workchain and compares against expected XML result.
+    """
     out_dir = build_sphinx(WORKCHAIN)
     index_file = join(out_dir, 'index.xml')
     if six.PY2:

aiida/sphinxext/tests/workchain_source/conf.py

@@ -27,9 +27,6 @@
 from __future__ import division
 from __future__ import print_function
 from __future__ import absolute_import
-import os
-import sys
-sys.path.insert(0, os.path.abspath('.'))
 
 # -- General configuration ------------------------------------------------
 

aiida/sphinxext/workchain.py

@@ -21,16 +21,18 @@
 from sphinx import addnodes
 from sphinx.ext.autodoc import ClassDocumenter
 
-from aiida.common.utils import get_object_from_string
 from plumpy.ports import OutputPort
+from aiida.common.utils import get_object_from_string
 
 
 def setup_aiida_workchain(app):
+    """Add sphinx directive."""
     app.add_directive_to_domain('py', 'aiida-workchain', AiidaWorkchainDirective)
     app.add_autodocumenter(WorkChainDocumenter)
 
 
 class WorkChainDocumenter(ClassDocumenter):
+    """Documenter class for AiiDA WorkChains."""
     directivetype = 'aiida-workchain'
     objtype = 'workchain'
     priority = 20
@@ -42,14 +44,12 @@ def can_document_member(cls, member, membername, isattr, parent):
             load_profile()
             from aiida.engine import WorkChain
             return issubclass(member, WorkChain)
-        except Exception:
+        except Exception:  # pylint: disable=broad-except
             return False
 
 
 class AiidaWorkchainDirective(Directive):
-    """
-    Directive to auto-document AiiDA workchains.
-    """
+    """Directive to auto-document AiiDA workchains."""
     required_arguments = 1
     optional_arguments = 0
     final_argument_whitespace = True
@@ -79,9 +79,7 @@ def load_workchain(self):
 
     def build_node_tree(self):
         """Returns the docutils node tree."""
-        workchain_node = addnodes.desc(
-            desctype='class', domain='py', noindex=False, objtype='class'
-        )
+        workchain_node = addnodes.desc(desctype='class', domain='py', noindex=False, objtype='class')
         workchain_node += self.build_signature()
         workchain_node += self.build_content()
         return [workchain_node]
@@ -102,11 +100,10 @@ def build_content(self):
         content += nodes.paragraph(text=self.workchain.__doc__)
 
         content += self.build_doctree(
-            title='Inputs:', port_namespace=self.workchain_spec.inputs,
-        )
-        content += self.build_doctree(
-            title='Outputs:', port_namespace=self.workchain_spec.outputs
+            title='Inputs:',
+            port_namespace=self.workchain_spec.inputs,
         )
+        content += self.build_doctree(title='Outputs:', port_namespace=self.workchain_spec.outputs)
 
         return content
 
@@ -117,7 +114,7 @@ def build_doctree(self, title, port_namespace):
         paragraph = nodes.paragraph()
         paragraph += nodes.strong(text=title)
         namespace_doctree = self.build_portnamespace_doctree(port_namespace)
-        if len(namespace_doctree) > 0:
+        if namespace_doctree:
             paragraph += namespace_doctree
         else:
             paragraph += nodes.paragraph(text='None defined.')
@@ -157,9 +154,7 @@ def build_port_paragraph(self, name, port):
         paragraph = nodes.paragraph()
         paragraph += addnodes.literal_strong(text=name)
         paragraph += nodes.Text(', ')
-        paragraph += nodes.emphasis(
-            text=self.format_valid_types(port.valid_type)
-        )
+        paragraph += nodes.emphasis(text=self.format_valid_types(port.valid_type))
         paragraph += nodes.Text(', ')
         paragraph += nodes.Text('required' if port.required else 'optional')
         if _is_non_db(port):
@@ -174,6 +169,12 @@ def build_port_paragraph(self, name, port):
 
     @staticmethod
     def format_valid_types(valid_type):
+        """
+        Format valid input/output types.
+
+        :param valid_type: class
+        :return: formatted string
+        """
         try:
             return valid_type.__name__
         except AttributeError:

setup.json

@@ -218,4 +218,4 @@
   "scripts": [
     "bin/runaiida"
   ]
-}
+}

utils/validate_consistency.py

@@ -186,10 +186,14 @@ def validate_version():
     Check version number in setup.json and aiida-core/__init__.py and make sure
     they match.
     """
+    import pkgutil
+
     # Get version from python package
-    sys.path.insert(0, ROOT_DIR)
-    import aiida  # pylint: disable=wrong-import-position
-    version = aiida.__version__
+    loaders = [
+        module_loader for (module_loader, name, ispkg) in pkgutil.iter_modules(path=[ROOT_DIR])
+        if name == 'aiida' and ispkg
+    ]
+    version = loaders[0].find_module('aiida').load_module('aiida').__version__
 
     setup_content = get_setup_json()
     if version != setup_content['version']: