[SciPy-Dev] New contribution: bode() function for LTI systems

Pauli Virtanen pav at iki.fi
Sat Jun 2 19:12:18 EDT 2012


02.06.2012 20:16, Bjørn Forsman kirjoitti:
[clip]
> https://github.com/scipy/scipy/pull/224 - ENH: ltisys: new bode() function
> https://github.com/scipy/scipy/pull/225 - ENH: ltisys: make lti zpk
> initialization work with plain lists

#225 is obvious, and should go in.

#224 -- looks mostly good. However, you don't include a test case, and
as I'm not familiar with the problem domain, I cannot easily
double-check that the code works (and therefore, cannot merge it).

Could you include a (simple but nontrivial) calculation where you know
the correct result, and that uses this code?  You most likely have
already done a couple of checks like this already, so coding up one or a
few of them as test cases hopefully is not too much work.

Having such tests in place is useful for Scipy for several reasons ---
for instance, it makes easier to check that new contributions work, and
having them in place makes it more certain that any future changes in
code (for instance in other parts of Scipy) do not break the functionality.

Normally, I or someone else of the regulars can of course write these up
themselves, but as Scipy spans a wide field of topics, it can take a
while until someone with the correct expertise has time for it... So,
it's really helpful if pull requests come with tests.

Thanks,

-- 
Pauli Virtanen




More information about the SciPy-Dev mailing list