Track offset in Signals

[jgosmann]

Description:
Makes Signals track the offset instead of computing it from the initial value.

Motivation and context:
See #1156, including this comment.

How has this been tested?
Added two unit tests. One is just dependent on the Signal and tests the offset value. The other one reproduces the original problem with the node input from #1156 (a bit simplified).

Where should a reviewer start?
Probably best to understand the problem first, then to look at the changes in signal.py.

How long should this take to review?

• Average (neither quick nor lengthy)

Types of changes:

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• I have read the CONTRIBUTING.rst document.
• I have updated the documentation accordingly. [no changes necessary]
• I have included a changelog entry.
• I have added tests to cover my changes.
• All new and existing tests passed.

[mention-bot]

@jgosmann, thanks for your PR! By analyzing the annotation information on this pull request, we identified @tbekolay to be a potential reviewer

[studywolf]

Where should a reviewer start?
Probably best to understand the problem first, then to look at the changes in signal.py.

Dude, c'mon. This isn't conducive to a friendly / helpful environment.

[jgosmann]

Honestly, it is not clear to me what is offensive about this sentence.

To clarify anyways: There are PRs where it is reasonable to dive directly into the diff. I don't think this is one of these PRs, it is probably easier to understand the changes after understanding the problem. That's why I suggest starting with that part first.

[studywolf]

It's not offensive, it just comes across as sarcastic / curt. Fair enough the comment describing the problem (which explains it quite well!) is linked in the motivation, but just reposting it in that section would be much more friendly / useful:

So, I figured out what's causing this (but I'm not sure how to fix it yet):
An array view is used as initial_value for a signal (that is not a view). From that signal a signal view is created which creates a view of the initial_value which results in a view of view of an array. But Numpy collapses this hierarchy to just one view of a signal. When initializing the signal dict the view offset will thus be calculated relative to the base array, but the (first) view of the array will be used for the data array and thus the offset is invalid.

I get sometimes people can dive straight into the diff, but suggesting people understand the problem to help understand the solution isn't really helpful 😜 It's just a tone thing too, eh? Emoji's help too 🎉 ✋ 🍰 😈

[jgosmann]

I suppose, I can see that it comes across as curt. Sometimes filling in all that information feels to be more work than the actual changes. That's certainly warranted in some cases, but with straight-forward changes it starts to annoy me. Sure, I could delete redundant/unhelpful sections in those cases, but to me the template imposes a psychological barrier to do so.

Anyways, I'll try to better myself.

[Seanny123]

LGTM.

[tbekolay]

Sorry it took so long for me to get to this one! Looked it over now and played around with NumPy enough so I understand it ;) Looks great! I made a few fixup commits for minor style and to add an __init__.py to builder/tests, and then a commit to move the builder tests to where they probably should have been all along. If these look to you @jgosmann I'll merge.

[jgosmann]

LGTM 🍰

[tbekolay]

TravisCI is complaining because flake8 recently updated with some new rules, so I'm going to sneak in a commit to fix those issues in this branch too.