GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Update evalfr() to be consistent with MATLAB usage by murrayrm · Pull Request #179 · python-control/python-control

Conversation

@murrayrm

@murrayrm murrayrm commented Jan 6, 2018

Copy link
Copy Markdown
Member

This PR addresses issue #22, in which it was pointed out that the evalfr() method in the TransferFunction and StateSpace objects takes different arguments than the evalfr() function. Specifically, the evalfr method takes a real argument (the frequency at which you want to evaluate the frequency response) and the evalfr function takes a complex argument (consistent with MATLAB usage).

This PR makes the following changes to the code base:

  • Renamed evalfr() in the FRD, StateSpace and TransferFunction classes to _evalfr() and put a deprecation warning for use of the evalfr() method.

  • Changed calls to evalfr() in frdata.py and margins.py to use _evalfr()

  • Added unit tests for the the new methods + warnings

These changes eliminate the inconsistency in the argument form between the (MATLAB compatible) evalfr() function and the evalfr() methods for LTI objects. The _evalfr() method retains the original functionality, since it is used for computing stability margins and for converting objects to FRD form.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
murrayrm added 5 commits January 2, 2018 08:04
@murrayrm
Merge branch 'master' of https://github.com/python-control/python-con…
558c1b9 
…trol
@murrayrm
Merge branch 'master' of https://github.com/python-control/python-con…
54d41aa 
…trol
@murrayrm
Merge branch 'master' of https://github.com/python-control/python-con…
9405886 
…trol
@murrayrm
Merge branch 'master' of https://github.com/python-control/python-con…
ee30983 
…trol
@murrayrm
Update evalfr usage for consistency with MATLAB
2915baf 
Renamed evalfr() in the FRD, StateSpace and TransferFunction classes to
_evalfr() and put a deprecation warning for use of the evalfr() method.

Changed calls to evalfr() in frdata.py and margins.py to use _evalfr()

Added unit tests for the the new methods + warnings

These changes eliminate the inconsistency in the argument form between the
(MATLAB compatible) evalfr() function, which takes a complex argument, and
the evalfr() methods for LTI objects, which took a real number as argument
(representing the frequency).  The _evalfr() method retains the original
functionality, since it is used for computing stability margins and for
converting objects to FRD form.
@coveralls

coveralls commented Jan 6, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 77.615% when pulling 2915baf on murrayrm:update_evalfr into 92ccf2d on python-control:master.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@murrayrm murrayrm mentioned this pull request Jan 6, 2018
evalfr does not behave as expected [sf#6] #22
Closed
@murrayrm
murrayrm merged commit bb65f31 into python-control:master Jan 13, 2018
@murrayrm
murrayrm deleted the update_evalfr branch January 13, 2018 06:35
@sawyerbfuller sawyerbfuller mentioned this pull request Aug 14, 2020
use __call__ instead of evalfr in lti system classes #449
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@murrayrm @coveralls
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.