Fix frequency argument in sine and multi_sine

[mgrub]

I just worked with the sine and multi_sine methods and the freq-argument gets used in different ways, than I would expect it:

In misc.testsignals.sine line 185 should be replaced by x = amp * np.sin(2 * np.pi * freq * time) (all multiplication). Alternativly, the freq parameter could be renamed to period.

In misc.testsignals.multi_sine line 213 should be replaced by x += amp * np.sin(2 * np.pi * freq * time) (scale sine by 2pi). Alternativly, the freq parameter could be renamed to angular_frequency.

To have a matching signature of both sine and multi_sine, I suggest to keep using the freq-argument, but adjust their use in the code as proposed.

Some references for the suggested changes can be found here:
https://de.wikipedia.org/wiki/Sinuston
https://en.wikipedia.org/wiki/Angular_frequency

[BjoernLudwigPTB]

I find your suggestions great . That's something we hadn't noticed yet! But this is again a breaking change, because the original calculations have to be changed, right? Should we add this to the "major release 2.0" milestone? By the way: shouldn't we rather call the sine function in misc.testsignals.multi_sine line 213 to avoid such a thing in the future?

[mgrub]

Yes, the original calculations have to be changed. But I am still not fully convinced, if this is a breaking change or a necessary fix (because we adjust the behaviour to what the docstring is already saying).

Yes, we should call sine from multisine.

[BjoernLudwigPTB]

Yes, the original calculations have to be changed. But I am still not fully convinced, if this is a breaking change or a necessary fix (because we adjust the behaviour to what the docstring is already saying).

mhm 🤨 you are right, still this changes the behaviour of previously written code. I can see how this reflects the expected behaviour though, following your linked articels. So you suggest introducing the changes and releasing this as a fix?

[mgrub]

Alright, I changed my mind. Let's consider this a breaking change and fix it with the 2.0.0 release.