[SciPy-Dev] Discrete-time additions to scipy.signal

[Ralf Gommers]

On Fri, Apr 29, 2011 at 2:39 PM, Jeffrey Armstrong <jba at sdf.lonestar.org> wrote:
> On Thu, 28 Apr 2011, Ralf Gommers wrote:
>
>> Are you trying to do this? If you need any help let me know (offline perhaps).
>
> I think I've successfully split the changes into two branches:
>
> discrete time additions:
> https://github.com/ArmstrongJ/scipy/tree/discrete-time-signal
>
> Lyapunov and algebraic Riccati solvers:
> https://github.com/ArmstrongJ/scipy/tree/riccati-lyapunov
>
> There could possibly be some minor mess with re-committing __init__.py in
> scipy.signal, but the final product is ok.  Git has taken a bit of time
> to understand.

That was a good start. I've taken your discrete-time-signal branch,
and did some more reorganizing to only have a few commits with no
renames and deletions:
https://github.com/rgommers/scipy/tree/armstrong-discrete. I then did
some cleanups of cont2discrete in the last commit, to make the code
cleaner.

Some comments on your code:
- function signatures with *args and **kwargs are usually a bad idea,
it is better to have separate functions or some other way to make
clear what the expected inputs are. (this is why I split up your
cont2discrete function).
- like Josef said, it should conform to PEP8.
- I also agree with him that it's better to not mix arrays and
matrices, and preferably avoid matrices completely.
- the standard numpy/scipy conventions for imports, docstrings etc.
should be followed, like "import numpy as np".  See also
https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt
- the permissions of your test files was wrong, they were executable.
Nose will ignore such files, so your tests were never executed.
- the caps in the test files are a little jarring to read. They can be
lowercase, and moved into the test class. Only the continuous
constants are reused, the discrete ones can be in the actual test
functions where they are used.

In the cont2discrete file, it would also be helpful if you added some
comments (or references) to the least transparent parts. For example,
I can't tell what the like with all the multiple vstack/hstack's is
doing.

I propose you pull my branch of your work and make changes to that.
Later the commits can be rewritten again to keep it easy to review.

Cheers,
Ralf