fixes problem reported in #434 and extends #179.

• move to __call__ as primary frequency response method to be used in TransferFunction, StateSpace, and FrequencyResponseData systems. (__call__ is new for StateSpace, and FrequencyResponse)
• the new __call__ has same interface for all three classes, and can now take one or an array of s, unlike the old _evalfr, which could only take one s
• it also adds a convenient keyword argument squeeze=True that automatically squeezes output if sys is SISO
• for FrequencyResponseData.__call__(s), s must be purely imaginary or an error is raised
• private method _evalfr that was inconsistent with matlab module matlab.evalfr was removed in favor of __call__
• method sys.evalfr(s) has been deprecated for 2.5 years and is now removed. use __call__ instead, or matlab.evalfr(sys, s) (following discussion in lti.evalfr and sys._evalfr have different behavior #434)
• method sys.freqresp(omega) is now deprecated. Use new pythonic methodsys.frequency_respose instead, or freqresp(sys, omega) from matlab module (following discussion in lti.evalfr and sys._evalfr have different behavior #434)
• horner(s) now does the same thing for both TransferFunction and StateSpace systems: can evaluate at multiple values of s
• de-duplication of code, cleanup, pythonization and vectorization of code where possible
• FRD.eval behavior was left as-is, except for an optional squeeze argument

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple of failing unit tests to still iron out

May I point your interest towards #438, where some of the evalfr/_evalfr/freqresp etc. test have been treated by the iron already. Especially some continuous/discrete inconsistency.

@bnavigator any idea how soon #438 will be ready for prime time? I can plan to try to merge (and figure out how to do that) when you’re done with the PR.

Hi,

I think it is ready. But it depends on the yet unmerged PRs mentioned in the initial post. We should get those approved and merged first. I understand #438 is hard to review. The diff will get smaller with the underlying PRs though.

Coverage decreased (-0.07%) to 87.438% when pulling a631098 on sawyerbfuller:call-method into 4103688 on python-control:master.

Coverage decreased (-0.2%) to 84.032% when pulling 2c4ac62 on sawyerbfuller:call-method into d3142ff on python-control:master.

A few comments: sys.horner was modified slightly so that it only takes care of the low-level algorithm, while __call__ is intended to perform user friendly tasks like input formatting and squeezing the output as necessary.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Co-authored-by: Ben Greiner <code@bnavigator.de>

Co-authored-by: Ben Greiner <code@bnavigator.de>

doc fix

Co-authored-by: Ben Greiner <code@bnavigator.de>

suggested doctoring fix for statesspace.slycot_horner

Co-authored-by: Ben Greiner <code@bnavigator.de>

doctoring fixes to adhere to numpydoc convention

Co-authored-by: Ben Greiner <code@bnavigator.de>

most recent commit moves the task of calling slycot and fall-back if it doesn't work into sys.horner

ok I think ive resolved doc comments with the last two commits

Travis isnt happy about somethng though.

Build 940 and 941 got canceled and 942 didn't start yet. We have to wait a little.

And the day when we can ditch Travis for a different CI will be a happy day for me. (Working on Github Actions for Slycot right now in python-control/Slycot#140, and then I will tackle #446)

Ok looks like Travis is happy now. I’m happy to have it merged but don’t understand the various options, happy to leave it to @murrayrm and @bnavigator discretion.