[MAINT] Cleanup EngineBase

[oesteban]

A quick cleanup of the EngineBase class:

• Added new tests for this base class
• Refactored test_utils.py, and moved some tests to test_nodes.py and test_workflows.py
• Cleaned up tests:
  • Avoid using lambdas when testing exceptions (not necessary anymore with pytest)
  • Parametrize test_workflows.py::test_outputs_removal_wf adding MultiProc and removing for loops

[effigies]

Isn't the point of having this before most imports to ensure that the imports work as expected?

[oesteban]

The aliases would modify the behavior of imports, so I guess that is the reason. Anyways, I moved it because flake8 suggested so. I can roll back at any moment

[effigies]

@mgxd @satra Are you familiar enough with future.standard_library.install_aliases to know whether there's a semantic change being made by reordering here?

You can also add a # noqa comment to pacify flake8, if you do return it.

[djarecka]

some comments on the order are here: https://pypi.python.org/pypi/future

All examples here have it at the very beginning

[effigies]

I would prefer:

if not name or not re.match(r'^\w+$', name):
    raise ValueError(...)
self._name = name

Note that I simplified the regex. \w matches underscores.

[oesteban]

I think we were supporting also dashes - so I'll add them.

[oesteban]

Testing with FMRIPREP this seems to work correctly.

[effigies]

I meant not to bother removing or reordering, and just tolerate the PEP8 violation. Unless it's causing problems, let's not change this here.

[oesteban]

I've checked locally with Python 2 and this aliases are not necessary. Let's get rid of them whenever possible.

[djarecka]

@oesteban @effigies I'm not sure if I understand, does standard_library.install_aliases() at the end of the import block (e.g. in node.py or workflow.py) changes anything?

[effigies]

I don't believe it does.

[djarecka]

we can change change Exception to TypeError

[oesteban]

Good to go!