Improve step and pipeline interface

[schustmi]

Pre-requisites

Please ensure you have done the following:

• I have read the CONTRIBUTING.md document.
• If my change requires a change to the documentation, I have updated the documentation accordingly.
• I have added tests to cover my changes.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Describe changes

• Add improved error handling when creating and calling steps
• Add tests for step definition and calling
• Replace SpecMaterializerRegistry with simple dictionary (it was nothing else to begin with)
• Add equality operations for post-execution classes (failed some tests locally because of random order I think)

[alex-zenml]

Nice! Loads of improvements to the error messages, plus all sorts of niceties like docstring improvements and of course a ton of tests to ensure it's doing what we think.

[alex-zenml]

Is it normal/standard to return NotImplemented in this way?

[schustmi]

Yes I think so, if I remember correctly returning NotImplemented in __eq__ is the best practice if you can't do the comparison yourself.

For example if someone decides to subclass ArtifactView (and doesn't do any major changes, let's say just add a small method to it), an object of that class might still be 'equal' to an ArtifactView object. But we can't check for that here in __eq__ as we don't know of the subclass.
If we return NotImplemented, python will as a next step try calling __eq__ on the other object (the subclass in our example, which might handle comparing to ArtifactView) and if that returns NotImplemented as well it will fallback to comparing if it's the same object in memory.

[alex-zenml]

Ok thank you. I didn't know that.

[alex-zenml]

No assertion in these tests. Not sure if that's best practice with pytest. I have a feeling these kinds of tests give a false feeling of security.

[schustmi]

No idea either, do you have any suggestions how we could improve it? Is there some pytest.no_raise or something similar?

[alex-zenml]

I've used the second style / syntax before in tests that I wrote:

https://stackoverflow.com/a/20275035/6649687

It's a bit wordier, but I find it a bit clearer and works better when it actually does fail.

[alex-zenml]

There is also this, but I think that's beyond what we need here.

[schustmi]

Actually I quite like the concept of the second link, but I would use it without the parametrize:

from contextlib import ExitStack as does_not_raise

def test_something():
    with does_not_raise():
        do_something()

What do you think about that? Maybe also @htahir1 @bcdurak

[alex-zenml]

Ok yes I like that better too.

[schustmi]

I added it for all step and pipeline tests

[htahir1]

I like it too

[htahir1]

Wow solid PR. Really like all the tests and could not think of more (Y)