diff --git a/.circleci/config.yml b/.circleci/config.yml index f0bfeaa0..be226fcf 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -23,9 +23,6 @@ jobs: # Run tests sudo python setup.py test - # Install requirements - sudo pip3 install pylint==2.3.0 - # Run pylint tests pylint setup.py find surround/ -iname "*.py" | xargs pylint diff --git a/examples/file-adapter/main.py b/examples/file-adapter/main.py index a2353e8c..776bd8e8 100644 --- a/examples/file-adapter/main.py +++ b/examples/file-adapter/main.py @@ -13,8 +13,7 @@ def load_data(self, mode, config): input_path = prefix + self.assembler.config.get_path("Surround.Loader.input") with open(input_path) as csv_file: - state.rows = csv.DictReader(csv_file, delimiter=',', quotechar='"') - state.rows = [row for row in state.rows] + state.rows = list(csv.DictReader(csv_file, delimiter=',', quotechar='"')) return state diff --git a/requirements.txt b/requirements.txt index 7e0ccebe..972174f2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,4 +3,5 @@ doit==0.31.1 pandas==0.25.2 numpy==1.17.3 tornado==6.0.2 -google-cloud-storage==1.20.0 \ No newline at end of file +google-cloud-storage==1.20.0 +pylint==2.4.3 \ No newline at end of file diff --git a/surround/cli.py b/surround/cli.py index d6c0eb7a..a79a094e 100644 --- a/surround/cli.py +++ b/surround/cli.py @@ -242,13 +242,11 @@ def parse_lint_args(parser, args, extra_args): linter = Linter() if args.list: - print(linter.dump_checks()) + linter.dump_checks() + elif remote_cli.get_project_root(os.path.abspath(args.path)): + linter.check_project(args.path, extra_args, verbose=True) else: - errors, warnings = linter.check_project(PROJECTS, args.path) - for e in errors + warnings: - print(e) - if not errors and not warnings: - print("All checks passed") + print("error: .surround does not exist") def parse_run_args(parser, args, extra_args): """ diff --git a/surround/config.py b/surround/config.py index 78f4e68e..b961bbb6 100644 --- a/surround/config.py +++ b/surround/config.py @@ -214,7 +214,7 @@ def __get_project_root(self, current_directory): parent_directory = os.path.dirname(current_directory) if current_directory in (home, parent_directory): break - elif ".surround" in list_: + if ".surround" in list_: return current_directory current_directory = parent_directory diff --git a/surround/data/util.py b/surround/data/util.py index ff81e2b0..9a955660 100644 --- a/surround/data/util.py +++ b/surround/data/util.py @@ -50,7 +50,8 @@ def prompt(question, required=True, answer_type=str, error_msg='Invalid answer, if answer == "" and required: print('This field is required!\n') continue - elif answer == "" and not required: + + if answer == "" and not required: print() return default diff --git a/surround/linter.py b/surround/linter.py index 01165599..dc2dbb68 100644 --- a/surround/linter.py +++ b/surround/linter.py @@ -1,234 +1,6 @@ import os -import io -from .stage import Filter, Estimator, Validator -from .state import State -from .assembler import Assembler - - -class LinterStage(Filter): - """ - Base class for a check in the Surround :class:`Linter`. - - Provides functions for creating warnings and errors that are - found during the linting process. - - Example:: - - class CheckExists(LinterStage): - def operate(self, data, config): - if not os.path.isdir(data.project_root): - self.add_error(data, "Project doesn't exist!") - """ - - def __init__(self, key, description): - """ - Constructor for a linter stage. - - :param key: identifier of the linter stage - :type key: str - :param description: short description of the linter stage - :type description: str - """ - - self.key = key - self.description = description - - def add_error(self, data, string): - """ - Creates an error which will be displayed and stop the :class:`Linter`. - - :param data: the data being passed between stages - :type data: :class:`ProjectData` - :param string: description of the error - :type string: str - """ - - data.errors.append("ERROR: %s_CHECK: %s" % (self.key, string)) - - def add_warning(self, data, string): - """ - Creates a warning that will be displayed but the :class:`Linter` will continue. - - :param data: the data being passed between stages - :type data: :class:`ProjectData` - :param string: description of the warning - :type string: str - """ - - data.warnings.append("WARNING: %s_CHECK: %s" % (self.key, string)) - - def operate(self, state, config): - """ - Executed by the :class:`Linter`, performs the linting specific to this stage. - **Must** be implemented in extended versions of this class. - - :param state: the data being passed between stages - :type state: :class:`ProjectData` - :param config: the configuration data for the linter - :type config: :class:`surround.config.Config` - """ - - -class CheckData(LinterStage): - """ - :class:`Linter` stage that checks the data folder in the surround project for files. - """ - - def __init__(self): - LinterStage.__init__(self, "DATA", "Check data files") - - def operate(self, state, config): - """ - Executed by the :class:`Linter`, checks if there is any files in the project's data folder. - If there is none then a warning will be issued. - - :param state: the data being passed between stages - :type state: :class:`surround.State` - :param config: the linter's configuration data - :type config: :class:`surround.config.Config` - """ - - path = os.path.join(state.project_root, "data") - if os.path.exists(path) and not os.listdir(path): - self.add_warning(state, "No data available, data directory is empty") - - -class CheckFiles(LinterStage): - """ - :class:`Linter` stage that checks the surround project files exist. - """ - - def __init__(self): - LinterStage.__init__(self, "FILES", "Check for Surround project files") - - def operate(self, state, config): - """ - Executed by the :class:`Linter`, checks if the files in the project structure exist. - Will create errors if required surround project files are missing in the root directory. - - :param state: the data being passed between stages - :type state: :class:`surround.State` - :param config: the linter's configuation data - :type config: :class:`surround.config.Config` - """ - - for result in state.project_structure["new"]["files"] + state.project_structure["new"]["templates"]: - file_name = result[0] - path = os.path.join( - state.project_root, - file_name.format(project_name=state.project_name)) - if not os.path.isfile(path): - self.add_error(state, "Path %s does not exist" % path) - - -class CheckDirectories(LinterStage): - """ - :class:`Linter` stage that checks the surround project directories exist. - """ - - def __init__(self): - LinterStage.__init__( - self, "DIRECTORIES", - "Check for validating Surround's directory structure") - - def operate(self, state, config): - """ - Executed by the :class:`Linter`, checks whether the project directories exist. - If the expected directories don't exist then errors will be created. - - :param state: the data being passed between stages - :type state: :class:`surround.State` - :param config: the linter's configuration data - :type config: :class:`surround.config.Config` - """ - - for d in state.project_structure["new"]["dirs"]: - path = os.path.join(state.project_root, - d.format(project_name=state.project_name)) - if not os.path.isdir(path): - self.add_error(state, "Directory %s does not exist" % path) - -class LinterValidator(Validator): - """ - Linter's validator stage, checks the data given in the ProjectData is valid. - """ - - def validate(self, state, config): - """ - Executed by the :class:`Linter`, checks whther the paths contained are valid. - - :param state: the data being passed between linter stages - :type state: :class:`surround.State` - :param config: the linter's configuration data - :type config: :class:`surround.config.Config` - """ - - if not isinstance(state.project_name, str): - state.errors.append("ERROR: PROJECT_CHECK: Project name is not a string") - - if not isinstance(state.project_structure, dict): - state.errors.append("ERROR: PROJECT_CHECK: Project structure invalid format") - - if not isinstance(state.project_root, str): - state.errors.append("ERROR: PROJECT_CHECK: Project root path is not a string") - -class Main(Estimator): - """ - Class responsible for executing all of the :class:`LinterStage`'s in the Surround Linter. - """ - - def __init__(self, filters): - """ - :param filters: list of stages in the linter - :type filters: list of :class:`LinterStage` - """ - - self.filters = filters - - def estimate(self, state, config): - """ - Execute each stage in the linter. - """ - - for filters in self.filters: - filters.operate(state, config) - - def fit(self, state, config): - """ - Should never be called. - """ - - print("No training implemented") - - -class ProjectData(State): - """ - Class containing the data passed between each :class:`LinterStage`. - - **Attributes:** - - - :attr:`project_structure` - expected file structure of the surround project (:class:`dict`) - - :attr:`project_root` - path to the root of the surround project (:class:`str`) - - :attr:`project_name` - name of the surround project (:class:`str`) - """ - - def __init__(self, project_structure, project_root, project_name): - """ - Constructor for the ProjectData class. - - :param project_structure: the expected file structure of the project - :type project_structure: dict - :param project_root: path to the root of the project - :type project_root: str - :param project_name: name of the project - :type project_name: str - """ - - super().__init__() - self.project_structure = project_structure - self.project_root = project_root - self.project_name = project_name - +from pathlib import Path +from pylint.lint import Run class Linter(): """ @@ -237,46 +9,50 @@ class Linter(): This class is used by the Surround CLI to perform the linting of a project via the `lint` sub-command. - - To add a new check to the linter, append an instance of it to the ``filters`` list. """ - filters = [CheckDirectories(), CheckFiles(), CheckData()] - def dump_checks(self): """ - Dumps a list of the checks in this linter. - The list is compiled using the :attr:`LinterStage.key` and :attr:`LinterStage.description` - attributes of each check. + Dumps a list of the checks in this linter to the terminal. :return: formatted list of the checkers in the linter :rtype: str """ - with io.StringIO() as s: - s.write("Checkers in Surround's linter\n") - s.write("=============================") - for stage in self.filters: - s.write("\n%s - %s" % (stage.key, stage.description)) - output = s.getvalue() - return output + print("Checkers in Surround's linter") + print("=============================") + + try: + Run(['--list-msgs-enabled']) + except SystemExit: + pass - def check_project(self, project, project_root=os.curdir): + def check_project(self, project_root=os.curdir, extra_args=None, verbose=False): """ - Runs the linter against the project specified, returning any warnings/errors. + Runs the linter against the project specified, returning zero on success - :param project: expected file structure of the project - :type project: dict :param project_root: path to the root of the project (default: current directory) :type project_root: str :return: errors and warnings found (if any) :rtype: (list of error strings, list of warning strings) """ - root = os.path.abspath(project_root) - project_name = os.path.basename(root) - data = ProjectData(project, root, project_name) - assembler = Assembler("Linting").set_validator(LinterValidator()).set_estimator(Main(self.filters)) - assembler.init_assembler() - assembler.run(data) - return data.errors, data.warnings + ignore_dirs = ['scripts', 'spikes', 'notebooks'] + args = [str(p) for p in Path(project_root).glob("**/*.py")] + args = [p for p in args if os.path.basename(os.path.dirname(p)) not in ignore_dirs] + + if extra_args: + args.extend(extra_args) + + disable_msgs = [ + 'missing-class-docstring', + 'missing-function-docstring', + 'abstract-method', + 'attribute-defined-outside-init' + ] + + for msg in disable_msgs: + args.append('--disable=%s' % msg) + + result = Run(args, do_exit=False) + return result.linter.msg_status == 0 diff --git a/surround/remote/cli.py b/surround/remote/cli.py index ef9b389e..a0d86695 100644 --- a/surround/remote/cli.py +++ b/surround/remote/cli.py @@ -49,7 +49,7 @@ def get_project_root(current_directory): parent_directory = os.path.dirname(current_directory) if current_directory in (home, parent_directory): break - elif ".surround" in list_: + if ".surround" in list_: return current_directory current_directory = parent_directory diff --git a/surround/tests/cli/data_cli/create_test.py b/surround/tests/cli/data_cli/create_test.py index bff57187..4c9d568d 100644 --- a/surround/tests/cli/data_cli/create_test.py +++ b/surround/tests/cli/data_cli/create_test.py @@ -40,7 +40,7 @@ def test_happy_path(self): std_input += "Test group description\n" std_input += "1\n" - process = subprocess.run(['surround', 'data', 'create', '-d', 'temp', '-o', 'temp.data.zip'], input=std_input, encoding='ascii') + process = subprocess.run(['surround', 'data', 'create', '-d', 'temp', '-o', 'temp.data.zip'], input=std_input, encoding='ascii', check=True) self.assertEqual(process.returncode, 0) self.assertTrue(os.path.exists("temp.data.zip")) diff --git a/surround/tests/cli/project_cli/generated_project_test.py b/surround/tests/cli/project_cli/generated_project_test.py index f0dc4939..61951a54 100644 --- a/surround/tests/cli/project_cli/generated_project_test.py +++ b/surround/tests/cli/project_cli/generated_project_test.py @@ -7,106 +7,106 @@ class InitTest(unittest.TestCase): def setUp(self): - subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE) + subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE, check=True) os.makedirs('remote') os.makedirs('temp/test_remote') Path('temp/test_remote/a.txt').touch() def test_run_from_subdir(self): - process = subprocess.run(['surround', 'run'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'run'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertEqual(process.stdout, "error: not a surround project\n") - process = subprocess.run(['surround', 'run'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'run'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertIn("batch", process.stdout) self.assertIn("Run batch mode inside the container", process.stdout) - process = subprocess.run(['surround', 'run'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'run'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) self.assertIn("batch", process.stdout) self.assertIn("Run batch mode inside the container", process.stdout) def test_remote_from_subdir(self): - process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertEqual(process.stdout, "error: not a surround project\n") - process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "info: no remote found\n") - process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) self.assertEqual(process.stdout, "info: no remote found\n") - process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'test_remote', '-u', '~'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') - process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'test_remote', '-u', '~'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) + process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) self.assertEqual(process.stdout, "test_remote\n") def test_pull_from_subdir(self): - process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertEqual(process.stdout, "error: not a surround project\n") - process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "error: no remote named test_remote\n") - process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) self.assertEqual(process.stdout, "error: no remote named test_remote\n") - process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'test_remote', '-u', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'test_remote', '-u', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) - process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "error: file does not exist\n") - process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) self.assertEqual(process.stdout, "error: file does not exist\n") - process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.txt'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.txt'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) self.assertRegex(process.stdout, r'info: test_remote[/\\]{1,2}a.txt already exists\n') def test_push_from_subdir(self): - process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertEqual(process.stdout, "error: not a surround project\n") - process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "error: no remote named test_remote\n") - process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) self.assertEqual(process.stdout, "error: no remote named test_remote\n") - process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'test_remote', '-u', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'test_remote', '-u', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) - process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "error: file does not exist\n") - process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.jpg'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) self.assertEqual(process.stdout, "error: file does not exist\n") - process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.txt'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.txt'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "info: a.txt pushed successfully\n") - process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.txt'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.txt'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) self.assertRegex(process.stdout, r'info: remote[/\\]{1,2}temp[/\\]{1,2}a.txt already exists\n') - process = subprocess.run(['rm', 'test_remote/a.txt'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') - process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.txt'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['rm', 'test_remote/a.txt'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) + process = subprocess.run(['surround', 'store', 'pull', 'test_remote', '-k', 'a.txt'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) self.assertEqual(process.stdout, "info: a.txt pulled successfully\n") def test_list_from_subdir(self): - process = subprocess.run(['surround', 'store', 'list', 'test_remote'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'store', 'list', 'test_remote'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertEqual(process.stdout, "error: not a surround project\n") - process = subprocess.run(['surround', 'store', 'list', 'test_remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'list', 'test_remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "error: no remote named test_remote\n") - process = subprocess.run(['surround', 'store', 'list', 'test_remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'list', 'test_remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) self.assertEqual(process.stdout, "error: no remote named test_remote\n") - process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'test_remote', '-u', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'test_remote', '-u', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) - process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.txt'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'push', 'test_remote', '-k', 'a.txt'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "info: a.txt pushed successfully\n") - process = subprocess.run(['surround', 'store', 'list', 'test_remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'list', 'test_remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "a.txt\n") - process = subprocess.run(['surround', 'store', 'list', 'test_remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp') + process = subprocess.run(['surround', 'store', 'list', 'test_remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp/temp', check=True) self.assertEqual(process.stdout, "a.txt\n") def tearDown(self): diff --git a/surround/tests/cli/project_cli/init_test.py b/surround/tests/cli/project_cli/init_test.py index e884bad8..6bd73c0a 100644 --- a/surround/tests/cli/project_cli/init_test.py +++ b/surround/tests/cli/project_cli/init_test.py @@ -9,13 +9,13 @@ class InitTest(unittest.TestCase): def test_happy_path(self): - process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertRegex(process.stdout, 'info: project created at .*temp\\n') is_temp = os.path.isdir(os.path.join(os.getcwd() + "/temp")) self.assertEqual(is_temp, True) - process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertEqual(process.stdout, "error: directory ./temp already exists\n") def tearDown(self): diff --git a/surround/tests/cli/project_cli/linter_test.py b/surround/tests/cli/project_cli/linter_test.py index 8d47ad5b..3acf0128 100644 --- a/surround/tests/cli/project_cli/linter_test.py +++ b/surround/tests/cli/project_cli/linter_test.py @@ -28,6 +28,7 @@ def test_lint_valid_project(self): # Linter should be able to locate .surround folder if project valid self.assertNotIn(".surround does not exist", output, "Current directory not a Surround project") + self.assertIn("Your code has been rated at 10.00/10", output, "The generated code is not lint error free!") def test_lint_invalid_project(self): """ diff --git a/surround/tests/cli/remote_cli/add_test.py b/surround/tests/cli/remote_cli/add_test.py index 1e264214..9654ecd2 100644 --- a/surround/tests/cli/remote_cli/add_test.py +++ b/surround/tests/cli/remote_cli/add_test.py @@ -11,7 +11,7 @@ class AddTest(unittest.TestCase): def test_rejecting_path(self): - process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertIn("info: project created at", process.stdout) self.assertIn("temp", process.stdout) @@ -25,7 +25,7 @@ def test_rejecting_path(self): os.chdir("../") self.assertEqual(result, "error: no remote named data") - process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'data', '-u', os.getcwd()], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'data', '-u', os.getcwd()], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) os.chdir("temp") result = local_remote.add('data', 'temp.jpg') @@ -33,14 +33,14 @@ def test_rejecting_path(self): self.assertEqual(result, "error: temp.jpg not found.") def test_happy_path(self): - process = subprocess.run(['surround', 'init', os.getcwd(), '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'init', os.getcwd(), '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertIn("info: project created at", process.stdout) self.assertIn("temp", process.stdout) is_temp = os.path.isdir(os.path.join(os.getcwd() + "/temp")) self.assertEqual(is_temp, True) - process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'data', '-u', os.getcwd()], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'data', '-u', os.getcwd()], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) local_remote = Local() diff --git a/surround/tests/cli/remote_cli/list_test.py b/surround/tests/cli/remote_cli/list_test.py index 97c3f989..6042b788 100644 --- a/surround/tests/cli/remote_cli/list_test.py +++ b/surround/tests/cli/remote_cli/list_test.py @@ -9,29 +9,29 @@ class ListTest(unittest.TestCase): def test_rejecting_path(self): - process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertRegex(process.stdout, 'info: project created at .*temp\\n') is_temp = os.path.isdir(os.path.join(os.getcwd() + "/temp")) self.assertEqual(is_temp, True) - process = subprocess.run(['surround', 'store', 'list', 'temp'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'list', 'temp'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "error: no remote named temp\n") def test_happy_path(self): - process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertRegex(process.stdout, 'info: project created at .*temp\\n') is_temp = os.path.isdir(os.path.join(os.getcwd() + "/temp")) self.assertEqual(is_temp, True) - process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'temp_remote', '-u', os.getcwd() + '/temp'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'temp_remote', '-u', os.getcwd() + '/temp'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, '') - process = subprocess.run(['surround', 'store', 'remote', '-v'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'remote', '-v'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertRegex(process.stdout, 'temp_remote: .*temp\\n') - process = subprocess.run(['surround', 'store', 'list', 'temp_remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'list', 'temp_remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertListEqual(sorted(str(process.stdout).splitlines()), sorted('config.yaml\n__main__.py\n__init__.py\nfile_system_runner.py\nweb_runner.py\nstages'.splitlines())) def tearDown(self): diff --git a/surround/tests/cli/remote_cli/remote_test.py b/surround/tests/cli/remote_cli/remote_test.py index 3d8d29e6..9a453153 100644 --- a/surround/tests/cli/remote_cli/remote_test.py +++ b/surround/tests/cli/remote_cli/remote_test.py @@ -9,31 +9,31 @@ class RemoteTest(unittest.TestCase): def test_remote(self): - process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertEqual(process.stdout, "error: not a surround project\n") - process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertRegex(process.stdout, 'info: project created at .*temp\\n') is_temp = os.path.isdir(os.path.join(os.getcwd() + "/temp")) self.assertEqual(is_temp, True) - process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "info: no remote found\n") def test_remote_add(self): - process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE) + process = subprocess.run(['surround', 'init', './', '-p', 'temp', '-d', 'temp', '-w', 'no'], encoding='utf-8', stdout=subprocess.PIPE, check=True) self.assertRegex(process.stdout, 'info: project created at .*temp\\n') is_temp = os.path.isdir(os.path.join(os.getcwd() + "/temp")) self.assertEqual(is_temp, True) - process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'data', '-u', os.getcwd()], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'remote', '-a', '-n', 'data', '-u', os.getcwd()], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) - process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'remote'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "data\n") - process = subprocess.run(['surround', 'store', 'remote', '-v'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp') + process = subprocess.run(['surround', 'store', 'remote', '-v'], encoding='utf-8', stdout=subprocess.PIPE, cwd='temp', check=True) self.assertEqual(process.stdout, "data: " + os.getcwd() + "\n") def tearDown(self): diff --git a/templates/new/assembler_state.py.txt b/templates/new/assembler_state.py.txt index f1fda47f..bf8ff61d 100644 --- a/templates/new/assembler_state.py.txt +++ b/templates/new/assembler_state.py.txt @@ -1,3 +1,8 @@ +""" +This module defines the state object that is passed between each stage +in the pipeline. +""" + from surround import State class AssemblerState(State): diff --git a/templates/new/baseline.py.txt b/templates/new/baseline.py.txt index 5d729a29..5ee8a144 100644 --- a/templates/new/baseline.py.txt +++ b/templates/new/baseline.py.txt @@ -1,3 +1,7 @@ +""" +This module defines the baseline estimator in the pipeline. +""" + import logging from surround import Estimator diff --git a/templates/new/batch_main.py.txt b/templates/new/batch_main.py.txt index 5cca5372..8c800212 100644 --- a/templates/new/batch_main.py.txt +++ b/templates/new/batch_main.py.txt @@ -1,15 +1,19 @@ +""" +Main entry-point for the Surround project. +Runners and assemblies are defined in here. +""" + import os import argparse from surround import Surround, Assembler, Config -from surround.experiment import ExperimentWriter -from stages import Baseline, InputValidator, ReportGenerator -from file_system_runner import FileSystemRunner +from .stages import Baseline, InputValidator, ReportGenerator +from .file_system_runner import FileSystemRunner -runners = [ +RUNNERS = [ FileSystemRunner() ] -assemblies = [ +ASSEMBLIES = [ Assembler("baseline") .set_validator(InputValidator()) .set_estimator(Baseline()) @@ -21,19 +25,42 @@ def main(): default_runner = config.get_path('runner.default') default_assembler = config.get_path('assembler.default') - parser = argparse.ArgumentParser(prog='{project_name}', description="Surround mode(s) available to run this module") - parser.set_defaults(experiment=True) - - parser.add_argument('-r', '--runner', help="Runner for the Assembler (index or name)", default=default_runner if default_runner is not None else "0") - parser.add_argument('-a', '--assembler', help="Assembler to run (index or name)", default=default_assembler if default_assembler is not None else "0") - parser.add_argument('-ne', '--no-experiment', dest='experiment', help="Don't consider this run an experiment", action="store_false") + parser = argparse.ArgumentParser( + prog='{project_name}', + description="Surround mode(s) available to run this module") + + parser.add_argument( + '-r', + '--runner', + help="Runner for the Assembler (index or name)", + default=default_runner if default_runner is not None else "0") + parser.add_argument( + '-a', + '--assembler', + help="Assembler to run (index or name)", + default=default_assembler if default_assembler is not None else "0") + parser.add_argument( + '-ne', + '--no-experiment', + dest='experiment', + help="Don't consider this run an experiment", + action="store_false") + parser.add_argument( + '--status', + help="Display information about the project such as available RUNNERS and assemblers", + action="store_true") parser.add_argument('-n', '--note', help="Add a note to the experiment", type=str) parser.add_argument('--mode', help="Mode to run (train, batch)", default="batch") - parser.add_argument('--status', help="Display information about the project such as available runners and assemblers", action="store_true") args = parser.parse_args() - surround = Surround(runners, assemblies, "{project_name}", "{project_description}", os.path.dirname(os.path.dirname(__file__))) + surround = Surround( + RUNNERS, + ASSEMBLIES, + "{project_name}", + "{project_description}", + os.path.dirname(os.path.dirname(__file__)) + ) if args.status: surround.show_info() diff --git a/templates/new/dodo.py.txt b/templates/new/dodo.py.txt index 5e74af7e..7bbcffba 100644 --- a/templates/new/dodo.py.txt +++ b/templates/new/dodo.py.txt @@ -1,3 +1,7 @@ +""" +This module defines the tasks that can be executed using `surround run [task name]` +""" + import os import sys import subprocess @@ -46,15 +50,35 @@ def task_remove(): def task_dev(): """Run the main task for the project""" + cmd = [ + "docker", + "run", + "--volume", + "\"%s/\":/app" % CONFIG["volume_path"], + "%s" % IMAGE, + "python3 -m %s %s" % (PACKAGE_PATH, "%(args)s") + ] return {{ - 'actions': ["docker run --volume \"%s/\":/app %s python3 -m %s %s" % (CONFIG["volume_path"], IMAGE, PACKAGE_PATH, "%(args)s")], + 'actions': [" ".join(cmd)], 'params': PARAMS }} def task_interactive(): """Run the Docker container in interactive mode""" def run(): - process = subprocess.Popen(['docker', 'run', '-it', '--rm', '-w', '/app', '--volume', '%s/:/app' % CONFIG['volume_path'], IMAGE, 'bash'], encoding='utf-8') + cmd = [ + 'docker', + 'run', + '-it', + '--rm', + '-w', + '/app', + '--volume', + '%s/:/app' % CONFIG['volume_path'], + IMAGE, + 'bash' + ] + process = subprocess.Popen(cmd, encoding='utf-8') process.wait() return {{ @@ -79,19 +103,30 @@ def task_train(): # Inject user's name and email into the env variables of the container user_name = global_config.get_path("user.name") user_email = global_config.get_path("user.email") - experiment_args = "-e \"SURROUND_USER_NAME=%s\" -e \"SURROUND_USER_EMAIL=%s\"" % (user_name, user_email) + experiment_args = "-e \"SURROUND_USER_NAME=%s\" " % user_name + experiment_args += "-e \"SURROUND_USER_EMAIL=%s\"" % user_email experiment_path = os.path.join(str(Path.home()), ".experiments") experiment_volume_path = generate_docker_volume_path(experiment_path) # Ensure experiments will work if using a local storage location (not in the cloud) if os.path.join(experiment_path, "local") == global_config.get_path("experiment.url"): - experiment_args += " --volume \"%s\":/experiments -e \"SURROUND_EXPERIMENT_URL=/experiments/local\"" % experiment_volume_path + experiment_args += " --volume \"%s\":/experiments " % experiment_volume_path + experiment_args += "-e \"SURROUND_EXPERIMENT_URL=/experiments/local\"" else: - experiment_args += " -e \"SURROUND_EXPERIMENT_URL=%s\"" % global_config.get_path("experiment.url") + current_url = global_config.get_path("experiment.url") + experiment_args += " -e \"SURROUND_EXPERIMENT_URL=%s\"" % current_url + + cmd = [ + "docker run %s" % experiment_args, + "--volume \"%s\":/app/output" % output_path, + "--volume \"%s\":/app/input" % data_path, + IMAGE, + "python3 -m {project_name} --mode train --experiment %(args)s" + ] return {{ - 'actions': ["docker run %s --volume \"%s\":/app/output --volume \"%s\":/app/input %s python3 -m {project_name} --mode train %s" % (experiment_args, output_path, data_path, IMAGE, "%(args)s")], + 'actions': [" ".join(cmd)], 'params': PARAMS }} @@ -105,58 +140,111 @@ def task_batch(): # Inject user's name and email into the env variables of the container user_name = global_config.get_path("user.name") user_email = global_config.get_path("user.email") - experiment_args = "-e \"SURROUND_USER_NAME=%s\" -e \"SURROUND_USER_EMAIL=%s\"" % (user_name, user_email) + experiment_args = "-e \"SURROUND_USER_NAME=%s\" " % user_name + experiment_args += "-e \"SURROUND_USER_EMAIL=%s\"" % user_email experiment_path = os.path.join(str(Path.home()), ".experiments") experiment_volume_path = generate_docker_volume_path(experiment_path) # Ensure experiments will work if using a local storage location (not in the cloud) if os.path.join(experiment_path, "local") == global_config.get_path("experiment.url"): - experiment_args += " --volume \"%s\":/experiments -e \"SURROUND_EXPERIMENT_URL=/experiments/local\"" % experiment_volume_path + experiment_args += " --volume \"%s\":/experiments " % experiment_volume_path + experiment_args += "-e \"SURROUND_EXPERIMENT_URL=/experiments/local\"" else: - experiment_args += " -e \"SURROUND_EXPERIMENT_URL=%s\"" % global_config.get_path("experiment.url") + current_url = global_config.get_path("experiment.url") + experiment_args += " -e \"SURROUND_EXPERIMENT_URL=%s\"" % current_url + + cmd = [ + "docker run %s" % experiment_args, + "--volume \"%s\":/app/output" % output_path, + "--volume \"%s\":/app/input" % data_path, + IMAGE, + "python3 -m {project_name} --mode batch %(args)s" + ] return {{ - 'actions': ["docker run %s --volume \"%s\":/app/output --volume \"%s\":/app/input %s python3 -m {project_name} --mode batch %s" % (experiment_args, output_path, data_path, IMAGE, "%(args)s")], + 'actions': [" ".join(cmd)], 'params': PARAMS }} def task_train_local(): """Run training mode locally""" + cmd = [ + sys.executable, + "-m %s" % PACKAGE_PATH, + "--mode train", + "%(args)s" + ] + return {{ 'basename': 'trainLocal', - 'actions': ["%s -m %s --mode train %s" % (sys.executable, PACKAGE_PATH, "%(args)s")], + 'actions': [" ".join(cmd)], 'params': PARAMS }} def task_batch_local(): """Run batch mode locally""" + cmd = [ + sys.executable, + "-m %s" % PACKAGE_PATH, + "--mode batch", + "%(args)s" + ] + return {{ 'basename': 'batchLocal', - 'actions': ["%s -m %s --mode batch %s" % (sys.executable, PACKAGE_PATH, "%(args)s")], + 'actions': [" ".join(cmd)], 'params': PARAMS }} def task_jupyter(): - command = "pip install -r /app/requirements.txt; mkdir /etc/ipython; echo \"c.InteractiveShellApp.extensions.append('autoreload')\nc.InteractiveShellApp.exec_lines = ['%autoreload 2', 'import sys', 'sys.path.append(\\'../\\')']\" > /etc/ipython/ipython_config.py; /usr/local/bin/start.sh jupyter notebook --NotebookApp.token=''" - + # Allow for auto reload to be enabled and import modules from project package + ipython_config = "c.InteractiveShellApp.extensions.append('autoreload')\n" + ipython_config += "c.InteractiveShellApp.exec_lines = " + ipython_config += "['%autoreload 2', 'import sys', 'sys.path.append(\\'../\\')']" + + # Build the command for running jupyter + command = [ + "pip install -r /app/requirements.txt", + "mkdir /etc/ipython", + "echo \"%s\" > /etc/ipython/ipython_config.py" % ipython_config, + "/usr/local/bin/start.sh jupyter notebook --NotebookApp.token=''" + ] + command = "; ".join(command) + def run_command(): - process = subprocess.Popen([ - "docker", - "run", - "--rm", - "--name", "{project_name}_surround_notebook", - "--volume", "%s:/app" % CONFIG['volume_path'], - "-p", "55910:8888", - "--user", "root", - "-w", "/app", - "jupyter/base-notebook:307ad2bb5fce", - "bash", "-c", command], stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8') + process = subprocess.Popen( + [ + "docker", + "run", + "--rm", + "--name", + "{project_name}_surround_notebook", + "--volume", + "%s:/app" % CONFIG['volume_path'], + "-p", + "55910:8888", + "--user", + "root", + "-w", + "/app", + "jupyter/base-notebook:307ad2bb5fce", + "bash", + "-c", + command + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + encoding='utf-8') print("Starting jupyter notbook server...\n") - + # Get the IP address of the container, otherwise use localhost - ip_process = subprocess.Popen(['docker-machine', 'ip'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8') + ip_process = subprocess.Popen( + ['docker-machine', 'ip'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + encoding='utf-8') ip_process.wait() ip_output = ip_process.stdout.readline().rstrip() @@ -168,14 +256,22 @@ def task_jupyter(): # Wait for the notebook server to be up before loading browser while True: - line = process.stderr.readline().rstrip() + line = process.stderr.readline().rstrip() if line and 'Serving notebooks from local directory' in line: break - elif process.poll(): + + if process.poll(): print("Failed to start the server, check if its not running somewhere else!") # Stop any containers that might be running - process = subprocess.Popen(['docker', 'stop', '{project_name}_surround_notebook'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + process = subprocess.Popen( + [ + 'docker', + 'stop', + '{project_name}_surround_notebook' + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) process.wait() return @@ -191,9 +287,16 @@ def task_jupyter(): pass finally: print("Closing server...") - process = subprocess.Popen(['docker', 'stop', '{project_name}_surround_notebook'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + process = subprocess.Popen( + [ + 'docker', + 'stop', + '{project_name}_surround_notebook' + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) process.wait() - + return {{ 'actions': [run_command] }} diff --git a/templates/new/file_system_runner.py.txt b/templates/new/file_system_runner.py.txt index c8a6a15b..529bd901 100644 --- a/templates/new/file_system_runner.py.txt +++ b/templates/new/file_system_runner.py.txt @@ -1,6 +1,10 @@ +""" +This module is responsible for loading the data and running the pipeline. +""" + import logging -from surround import Runner, RunMode -from stages import AssemblerState +from surround import Runner +from .stages import AssemblerState logging.basicConfig(level=logging.INFO) diff --git a/templates/new/init.py.txt b/templates/new/init.py.txt index cc2c489b..4cfc0097 100644 --- a/templates/new/init.py.txt +++ b/templates/new/init.py.txt @@ -1,3 +1,7 @@ +""" +This file is required for the project to be considered a python package. +""" + import os import sys diff --git a/templates/new/input_validator.py.txt b/templates/new/input_validator.py.txt index da4ef782..91bf58a5 100644 --- a/templates/new/input_validator.py.txt +++ b/templates/new/input_validator.py.txt @@ -1,3 +1,9 @@ +""" +This module defines the input validator which is executed +before all other stages in the pipeline and checks whether +the data contained in the State object is valid. +""" + from surround import Validator class InputValidator(Validator): diff --git a/templates/new/report_generator.py.txt b/templates/new/report_generator.py.txt index d7f78330..6ba69d41 100644 --- a/templates/new/report_generator.py.txt +++ b/templates/new/report_generator.py.txt @@ -1,7 +1,13 @@ +""" +This module generates the HTML report which uses the +template in the templates folder. This report can then +be viewed in the experimentation web application. +""" + import os -import surround import tornado.template +import surround from surround.experiment.util import get_surround_config class ReportGenerator(surround.Visualiser): @@ -11,7 +17,11 @@ class ReportGenerator(surround.Visualiser): self.global_config = get_surround_config() def visualise(self, state, config): - results_page = self.results_template.generate(config=config, state=state, global_config=self.global_config, surround_version=surround.__version__) + results_page = self.results_template.generate( + config=config, + state=state, + global_config=self.global_config, + surround_version=surround.__version__) - with open(os.path.join(config["output_path"], "results.html"), "wb+") as f: - f.write(results_page) + with open(os.path.join(config["output_path"], "results.html"), "wb+") as out: + out.write(results_page) diff --git a/templates/new/stages_init.py.txt b/templates/new/stages_init.py.txt index 3441b98f..338ffde3 100644 --- a/templates/new/stages_init.py.txt +++ b/templates/new/stages_init.py.txt @@ -1,3 +1,8 @@ +""" +Import each stage you create in this file so they can be imported +directly from the stages package e.g. `from .stages import Baseline`. +""" + from .baseline import Baseline from .input_validator import InputValidator from .assembler_state import AssemblerState diff --git a/templates/new/web_dodo.py.txt b/templates/new/web_dodo.py.txt index 89c09171..fa3fbe16 100644 --- a/templates/new/web_dodo.py.txt +++ b/templates/new/web_dodo.py.txt @@ -1,3 +1,7 @@ +""" +This module defines the tasks that can be executed using `surround run [task name]` +""" + import os import sys import subprocess @@ -46,15 +50,38 @@ def task_remove(): def task_dev(): """Run the main task for the project""" + cmd = [ + "docker", + "run", + "-p 8080:8080", + "--volume", + "\"%s/\":/app" % CONFIG["volume_path"], + "%s" % IMAGE, + "python3 -m %s %s" % (PACKAGE_PATH, "%(args)s") + ] return {{ - 'actions': ["docker run -p 8080:8080 --volume \"%s/\":/app %s python3 -m %s %s" % (CONFIG["volume_path"], IMAGE, PACKAGE_PATH, "%(args)s")], + 'actions': [" ".join(cmd)], 'params': PARAMS }} def task_interactive(): """Run the Docker container in interactive mode""" def run(): - process = subprocess.Popen(['docker', 'run', '-p', '8080:8080', '-it', '--rm', '-w', '/app', '--volume', '%s/:/app' % CONFIG['volume_path'], IMAGE, 'bash'], encoding='utf-8') + cmd = [ + 'docker', + 'run', + '-p', + '8080:8080', + '-it', + '--rm', + '-w', + '/app', + '--volume', + '%s/:/app' % CONFIG['volume_path'], + IMAGE, + 'bash' + ] + process = subprocess.Popen(cmd, encoding='utf-8') process.wait() return {{ @@ -63,8 +90,17 @@ def task_interactive(): def task_prod(): """Run the main task inside a Docker container for use in production """ + cmd = [ + "docker", + "run", + "-p 8080:8080", + IMAGE, + "python3 -m %s" % PACKAGE_PATH, + "%(args)s" + ] + return {{ - 'actions': ["docker run -p 8080:8080 %s python3 -m %s %s" % (IMAGE, PACKAGE_PATH, "%(args)s")], + 'actions': [" ".join(cmd)], 'task_dep': ["build"], 'params': PARAMS }} @@ -79,19 +115,30 @@ def task_train(): # Inject user's name and email into the env variables of the container user_name = global_config.get_path("user.name") user_email = global_config.get_path("user.email") - experiment_args = "-e \"SURROUND_USER_NAME=%s\" -e \"SURROUND_USER_EMAIL=%s\"" % (user_name, user_email) + experiment_args = "-e \"SURROUND_USER_NAME=%s\" " % user_name + experiment_args += "-e \"SURROUND_USER_EMAIL=%s\"" % user_email experiment_path = os.path.join(str(Path.home()), ".experiments") experiment_volume_path = generate_docker_volume_path(experiment_path) # Ensure experiments will work if using a local storage location (not in the cloud) if os.path.join(experiment_path, "local") == global_config.get_path("experiment.url"): - experiment_args += " --volume \"%s\":/experiments -e \"SURROUND_EXPERIMENT_URL=/experiments/local\"" % experiment_volume_path + experiment_args += " --volume \"%s\":/experiments " % experiment_volume_path + experiment_args += "-e \"SURROUND_EXPERIMENT_URL=/experiments/local\"" else: - experiment_args += " -e \"SURROUND_EXPERIMENT_URL=%s\"" % global_config.get_path("experiment.url") + current_url = global_config.get_path("experiment.url") + experiment_args += " -e \"SURROUND_EXPERIMENT_URL=%s\"" % current_url + + cmd = [ + "docker run %s" % experiment_args, + "--volume \"%s\":/app/output" % output_path, + "--volume \"%s\":/app/input" % data_path, + "%s" % IMAGE, + "python3 -m {project_name} --mode train %(args)s" + ] return {{ - 'actions': ["docker run %s --volume \"%s\":/app/output --volume \"%s\":/app/input %s python3 -m {project_name} --mode train --runner 1 %s" % (experiment_args, output_path, data_path, IMAGE, "%(args)s")], + 'actions': [" ".join(cmd)], 'params': PARAMS }} @@ -105,74 +152,147 @@ def task_batch(): # Inject user's name and email into the env variables of the container user_name = global_config.get_path("user.name") user_email = global_config.get_path("user.email") - experiment_args = "-e \"SURROUND_USER_NAME=%s\" -e \"SURROUND_USER_EMAIL=%s\"" % (user_name, user_email) + experiment_args = "-e \"SURROUND_USER_NAME=%s\" " % user_name + experiment_args += "-e \"SURROUND_USER_EMAIL=%s\"" % user_email experiment_path = os.path.join(str(Path.home()), ".experiments") experiment_volume_path = generate_docker_volume_path(experiment_path) # Ensure experiments will work if using a local storage location (not in the cloud) if os.path.join(experiment_path, "local") == global_config.get_path("experiment.url"): - experiment_args += " --volume \"%s\":/experiments -e \"SURROUND_EXPERIMENT_URL=/experiments/local\"" % experiment_volume_path + experiment_args += " --volume \"%s\":/experiments " % experiment_volume_path + experiment_args += "-e \"SURROUND_EXPERIMENT_URL=/experiments/local\"" else: - experiment_args += " -e \"SURROUND_EXPERIMENT_URL=%s\"" % global_config.get_path("experiment.url") + current_url = global_config.get_path("experiment.url") + experiment_args += " -e \"SURROUND_EXPERIMENT_URL=%s\"" % current_url + + cmd = [ + "docker run %s" % experiment_args, + "--volume \"%s\":/app/output" % output_path, + "--volume \"%s\":/app/input" % data_path, + "%s" % IMAGE, + "python3 -m {project_name} --mode batch %(args)s" + ] return {{ - 'actions': ["docker run %s --volume \"%s\":/app/output --volume \"%s\":/app/input %s python3 -m {project_name} --mode batch --runner 1 %s" % (experiment_args, output_path, data_path, IMAGE, "%(args)s")], + 'actions': [" ".join(cmd)], 'params': PARAMS }} def task_train_local(): """Run training mode locally""" + cmd = [ + sys.executable, + "-m %s" % PACKAGE_PATH, + "--mode train", + "--runner 1", + "%(args)s" + ] + return {{ 'basename': 'trainLocal', - 'actions': ["%s -m %s --mode train --runner 1 %s" % (sys.executable, PACKAGE_PATH, "%(args)s")], + 'actions': [" ".join(cmd)], 'params': PARAMS }} def task_batch_local(): """Run batch mode locally""" + cmd = [ + sys.executable, + "-m %s" % PACKAGE_PATH, + "--mode batch", + "--runner 1", + "%(args)s" + ] + return {{ 'basename': 'batchLocal', - 'actions': ["%s -m %s --mode batch --runner 1 %s" % (sys.executable, PACKAGE_PATH, "%(args)s")], + 'actions': [" ".join(cmd)], 'params': PARAMS }} def task_web(): """Run web mode inside the container""" + cmd = [ + "docker", + "run", + "-p", + "8080:8080", + IMAGE, + "python3 -m %s" % PACKAGE_PATH, + "--runner WebRunner", + "%(args)s" + ] + return {{ - 'actions': ["docker run -p 8080:8080 %s python3 -m %s --runner WebRunner %s" % (IMAGE, PACKAGE_PATH, "%(args)s")], + 'actions': [" ".join(cmd)], 'params': PARAMS }} def task_web_local(): """Run web mode locally""" + cmd = [ + sys.executable, + "-m %s" % PACKAGE_PATH, + "--runner WebRunner", + "%(args)s" + ] + return {{ 'basename': 'webLocal', - 'actions': ["%s -m %s --runner WebRunner %s" % (sys.executable, PACKAGE_PATH, "%(args)s")], + 'actions': [" ".join(cmd)], 'params': PARAMS }} def task_jupyter(): - command = "pip install -r /app/requirements.txt; mkdir /etc/ipython; echo \"c.InteractiveShellApp.extensions.append('autoreload')\nc.InteractiveShellApp.exec_lines = ['%autoreload 2', 'import sys', 'sys.path.append(\\'../\\')']\" > /etc/ipython/ipython_config.py; /usr/local/bin/start.sh jupyter notebook --NotebookApp.token=''" - + # Allow for auto reload to be enabled and import modules from project package + ipython_config = "c.InteractiveShellApp.extensions.append('autoreload')\n" + ipython_config += "c.InteractiveShellApp.exec_lines = " + ipython_config += "['%autoreload 2', 'import sys', 'sys.path.append(\\'../\\')']" + + # Build the command for running jupyter + command = [ + "pip install -r /app/requirements.txt", + "mkdir /etc/ipython", + "echo \"%s\" > /etc/ipython/ipython_config.py" % ipython_config, + "/usr/local/bin/start.sh jupyter notebook --NotebookApp.token=''" + ] + command = "; ".join(command) + def run_command(): - process = subprocess.Popen([ - "docker", - "run", - "--rm", - "--name", "{project_name}_surround_notebook", - "--volume", "%s:/app" % CONFIG['volume_path'], - "-p", "55910:8888", - "--user", "root", - "-w", "/app", - "jupyter/base-notebook:307ad2bb5fce", - "bash", "-c", command], stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8') + process = subprocess.Popen( + [ + "docker", + "run", + "--rm", + "--name", + "{project_name}_surround_notebook", + "--volume", + "%s:/app" % CONFIG['volume_path'], + "-p", + "55910:8888", + "--user", + "root", + "-w", + "/app", + "jupyter/base-notebook:307ad2bb5fce", + "bash", + "-c", + command + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + encoding='utf-8') print("Starting jupyter notbook server...\n") - + # Get the IP address of the container, otherwise use localhost - ip_process = subprocess.Popen(['docker-machine', 'ip'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8') + ip_process = subprocess.Popen( + ['docker-machine', 'ip'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + encoding='utf-8') ip_process.wait() ip_output = ip_process.stdout.readline().rstrip() @@ -184,14 +304,22 @@ def task_jupyter(): # Wait for the notebook server to be up before loading browser while True: - line = process.stderr.readline().rstrip() + line = process.stderr.readline().rstrip() if line and 'Serving notebooks from local directory' in line: break - elif process.poll(): + + if process.poll(): print("Failed to start the server, please try again!") # Stop any containers that might be running - process = subprocess.Popen(['docker', 'stop', '{project_name}_surround_notebook'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + process = subprocess.Popen( + [ + 'docker', + 'stop', + '{project_name}_surround_notebook' + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) process.wait() return @@ -207,9 +335,16 @@ def task_jupyter(): pass finally: print("Closing server...") - process = subprocess.Popen(['docker', 'stop', '{project_name}_surround_notebook'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + process = subprocess.Popen( + [ + 'docker', + 'stop', + '{project_name}_surround_notebook' + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) process.wait() - + return {{ 'actions': [run_command] }} diff --git a/templates/new/web_main.py.txt b/templates/new/web_main.py.txt index f4e2e9ca..7de7cb84 100644 --- a/templates/new/web_main.py.txt +++ b/templates/new/web_main.py.txt @@ -1,17 +1,21 @@ +""" +Main entry-point for the Surround project. +Runners and ASSEMBLIES are defined in here. +""" + import os import argparse from surround import Surround, Assembler, Config -from surround.experiment import ExperimentWriter -from stages import Baseline, InputValidator, ReportGenerator -from file_system_runner import FileSystemRunner -from web_runner import WebRunner +from .stages import Baseline, InputValidator, ReportGenerator +from .file_system_runner import FileSystemRunner +from .web_runner import WebRunner -runners = [ +RUNNERS = [ WebRunner(), FileSystemRunner() ] -assemblies = [ +ASSEMBLIES = [ Assembler("baseline") .set_validator(InputValidator()) .set_estimator(Baseline()) @@ -23,19 +27,42 @@ def main(): default_runner = config.get_path('runner.default') default_assembler = config.get_path('assembler.default') - parser = argparse.ArgumentParser(prog='{project_name}', description="Surround mode(s) available to run this module") - parser.set_defaults(experiment=True) - - parser.add_argument('-r', '--runner', help="Runner for the Assembler (index or name)", default=default_runner if default_runner is not None else "0") - parser.add_argument('-a', '--assembler', help="Assembler to run (index or name)", default=default_assembler if default_assembler is not None else "0") - parser.add_argument('-ne', '--no-experiment', dest='experiment', help="Don't consider this run an experiment", action="store_false") + parser = argparse.ArgumentParser( + prog='{project_name}', + description="Surround mode(s) available to run this module") + + parser.add_argument( + '-r', + '--runner', + help="Runner for the Assembler (index or name)", + default=default_runner if default_runner is not None else "0") + parser.add_argument( + '-a', + '--assembler', + help="Assembler to run (index or name)", + default=default_assembler if default_assembler is not None else "0") + parser.add_argument( + '-ne', + '--no-experiment', + dest='experiment', + help="Don't consider this run an experiment", + action="store_false") + parser.add_argument( + '--status', + help="Display information about the project such as available RUNNERS and assemblers", + action="store_true") parser.add_argument('-n', '--note', help="Add a note to the experiment", type=str) parser.add_argument('--mode', help="Mode to run (train, batch)", default="batch") - parser.add_argument('--status', help="Display information about the project such as available runners and assemblers", action="store_true") args = parser.parse_args() - surround = Surround(runners, assemblies, "{project_name}", "{project_description}", os.path.dirname(os.path.dirname(__file__))) + surround = Surround( + RUNNERS, + ASSEMBLIES, + "{project_name}", + "{project_description}", + os.path.dirname(os.path.dirname(__file__)) + ) if args.status: surround.show_info() diff --git a/templates/new/web_runner.py.txt b/templates/new/web_runner.py.txt index b9ab2e3a..b7c75e66 100644 --- a/templates/new/web_runner.py.txt +++ b/templates/new/web_runner.py.txt @@ -1,3 +1,7 @@ +""" +This module is responsible for serving the pipeline via HTTP endpoints. +""" + import json import logging import tornado.httpserver @@ -5,7 +9,7 @@ import tornado.ioloop import tornado.options import tornado.web from surround import Runner, RunMode -from stages import AssemblerState +from .stages import AssemblerState logging.basicConfig(level=logging.INFO) @@ -35,20 +39,16 @@ class Application(tornado.web.Application): class EstimateHandler(tornado.web.RequestHandler): def initialize(self, assembler): self.assembler = assembler - self.data = AssemblerState() def post(self): req_data = json.loads(self.request.body) - # Clean output_data on every request - self.data.output_data = "" - - # Prepare input_date for the assembler - self.data.input_data = req_data["message"] + # Prepare input_data for the assembler + data = AssemblerState(req_data["message"]) # Execute assembler - self.assembler.run(self.data) - logging.info("Message: %s", self.data.output_data) + self.assembler.run(data) + logging.info("Message: %s", data.output_data) class InfoHandler(tornado.web.RequestHandler): @@ -56,4 +56,4 @@ class InfoHandler(tornado.web.RequestHandler): self.assembler = assembler def get(self): - self.write({{'version': '0.0.1'}}) + self.write({{'version': '0.0.1'}})