Merge pull request #43 from LSSTDESC/issue_42

add python_paths option to config

ceci/main.py

@@ -5,6 +5,7 @@
 import subprocess
 from . import pipeline
 from .sites import load, set_default_site, get_default_site
+from .utils import extra_paths
 
 # Add the current dir to the path - often very useful
 sys.path.append(os.getcwd())
@@ -108,8 +109,6 @@ def run(pipeline_config_filename, extra_config=None, dry_run=False):
         "resume": pipe_config["resume"],
     }
 
-    for module in modules:
-        __import__(module)
 
     # Choice of actual pipeline type to run
     if dry_run:
@@ -123,39 +122,54 @@ def run(pipeline_config_filename, extra_config=None, dry_run=False):
     else:
         raise ValueError("Unknown pipeline launcher {launcher_name}")
 
-    # Run the pre-script.  Since it's an error for this to fail (because
-    # it is useful as a validation check) then we raise an error if it
-    # fails using check_call.
-    if pre_script and not dry_run:
-        subprocess.check_call(pre_script.split() + script_args, shell=True)
-
-    # Create and run the pipeline
-    p = pipeline_class(stages, launcher_config)
-    status = p.run(inputs, run_config, stages_config)
-
-    # The load command above changes the default site.
-    # So that this function doesn't confuse later things,
-    # reset that site now.
-    set_default_site(default_site)
-
-    if status:
-        return status
-
-    # Run the post-script.  There seems less point raising an actual error
-    # here, as the pipeline is complete, so we just issue a warning and
-    # return a status code to the caller (e.g. to the command line).
-    # Thoughts on this welcome.
-    if post_script and not dry_run:
-        return_code = subprocess.call(post_script.split() + script_args, shell=True)
-        if return_code:
-            sys.stderr.write(
-                f"\nWARNING: The post-script command {post_script} "
-                "returned error status {return_code}\n\n"
-            )
-        return return_code
-    # Otherwise everything must have gone fine.
-    else:
-        return status
+
+
+
+    paths = pipe_config.get("python_paths", [])
+    if isinstance(paths, str):
+        paths = paths.split()
+
+    # temporarily add the paths to sys.path,
+    # but remove them at the end
+    with extra_paths(paths):
+
+        for module in modules:
+            __import__(module)
+
+        # Run the pre-script.  Since it's an error for this to fail (because
+        # it is useful as a validation check) then we raise an error if it
+        # fails using check_call.
+        if pre_script and not dry_run:
+            subprocess.check_call(pre_script.split() + script_args, shell=True)
+
+        # Create and run the pipeline
+        try:
+            p = pipeline_class(stages, launcher_config)
+            status = p.run(inputs, run_config, stages_config)
+        finally:
+            # The load command above changes the default site.
+            # So that this function doesn't confuse later things,
+            # reset that site now.
+            set_default_site(default_site)
+
+        if status:
+            return status
+
+        # Run the post-script.  There seems less point raising an actual error
+        # here, as the pipeline is complete, so we just issue a warning and
+        # return a status code to the caller (e.g. to the command line).
+        # Thoughts on this welcome.
+        if post_script and not dry_run:
+            return_code = subprocess.call(post_script.split() + script_args, shell=True)
+            if return_code:
+                sys.stderr.write(
+                    f"\nWARNING: The post-script command {post_script} "
+                    "returned error status {return_code}\n\n"
+                )
+            return return_code
+        # Otherwise everything must have gone fine.
+        else:
+            return status
 
 
 def override_config(config, extra):

ceci/utils.py

@@ -0,0 +1,55 @@
+from contextlib import contextmanager
+import sys
+
+@contextmanager
+def extra_paths(paths, start=True):
+    # allow passing a single path or
+    # a list of them
+    if isinstance(paths, str):
+        paths = paths.split()
+
+    # On enter, add paths to sys.path,
+    # either the start or the end depending
+    # on the start argument
+    for path in paths:
+        if start:
+            sys.path.insert(0, path)
+        else:
+            sys.path.append(path)
+
+    # Return control to caller
+    try:
+        yield
+    # On exit, remove the paths
+    finally:
+        for path in paths:
+            try:
+                if start:
+                    sys.path.remove(path)
+                else:
+                    remove_last(sys.path, path)
+            # If e.g. user has already done this
+            # manually for some reason then just
+            # skip
+            except ValueError:
+                pass
+
+def remove_last(lst, item):
+    """
+    Removes (in-place) the last instance of item from the list lst.
+    Raises ValueError if item is not in list
+
+    Parameters
+    ----------
+    lst: List
+        A list of anything
+    item: object
+        Item to be removed
+
+    Returns
+    -------
+    None
+    """
+    tmp = lst[::-1]
+    tmp.remove(item)
+    lst[:] = tmp[::-1]

tests/test_python_paths.py

@@ -0,0 +1,127 @@
+import sys
+from ceci.main import run
+from ceci.utils import remove_last, extra_paths
+import pytest
+
+def test_remove_item():
+    l = list('abcdea')
+    remove_last(l, 'a')
+    assert l == list('abcde')
+
+    l = list('abcde')
+    remove_last(l, 'b')
+    assert l == list('acde')
+
+    with pytest.raises(ValueError):
+        remove_last([1, 2 ,3], 4)
+
+class MyError(Exception):
+    pass
+
+def test_extra_paths():
+    p = 'xxx111yyy222'
+    orig_path = sys.path[:]
+
+    # check path is put in
+    with extra_paths(p):
+        assert sys.path[0] == p
+
+    # check everything back to normal
+    # after with statement
+    assert p not in sys.path
+    assert sys.path == orig_path
+
+    # check that an exception does not interfere
+    # with this
+    try:
+        with extra_paths(p):
+            assert sys.path[0] == p
+            raise MyError("x")
+    except MyError:
+        pass
+
+    assert p not in sys.path
+    assert sys.path == orig_path
+
+
+    # now putting the item at the end not the start
+    with extra_paths(p, start=False):
+        assert sys.path[-1] == p
+
+    assert p not in sys.path
+    assert sys.path == orig_path
+
+    try:
+        with extra_paths(p, start=False):
+            assert sys.path[-1] == p
+            raise MyError("x")
+    except MyError:
+        pass
+
+    assert p not in sys.path
+    assert sys.path == orig_path
+
+    # now agan with a list of paths
+    p = ['xxx111yyy222', 'aaa222333']
+    with extra_paths(p):
+        assert sys.path[0] == p[1]
+        assert sys.path[1] == p[0]
+
+    for p1 in p:
+        assert p1 not in sys.path
+    assert sys.path == orig_path
+
+    try:
+        with extra_paths(p):
+            assert sys.path[0] == p[1]
+            assert sys.path[1] == p[0]
+            raise MyError("x")
+    except MyError:
+        pass
+
+    for p1 in p:
+        assert p1 not in sys.path
+    assert sys.path == orig_path
+
+
+
+    # now agan with a list of paths, at the end
+    p = ['xxx111yyy222', 'aaa222333']
+    with extra_paths(p, start=False):
+        assert sys.path[-1] == p[1]
+        assert sys.path[-2] == p[0]
+
+    for p1 in p:
+        assert p1 not in sys.path
+    assert sys.path == orig_path
+
+    try:
+        with extra_paths(p, start=False):
+            assert sys.path[-1] == p[1]
+            assert sys.path[-2] == p[0]
+            raise MyError("x")
+    except MyError:
+        pass
+
+    assert p not in sys.path
+    assert sys.path == orig_path
+
+    # check that if the user removes the path
+    # themselves then it is okay
+    p = ['xxx111yyy222', 'aaa222333']
+    with extra_paths(p, start=True):
+        sys.path.remove('xxx111yyy222')
+
+    assert sys.path == orig_path
+
+    # check only one copy is removed
+    sys.path.append("aaa")
+    tmp_paths = sys.path[:]
+    p = "aaa"
+    with extra_paths(p, start=True):
+        pass
+
+    assert sys.path == tmp_paths
+
+    with extra_paths(p, start=False):
+        pass