Thanks for this contribution, @SCLiao47.

I think the _bandwidth method should be renamed to bandwidth and put in the LTI class in lti.py. With that change, you should be able to get rid of the bandwidth methods in TransferFunction and StateSpace, since they will get the method from their parent class. You will probably have to override bandwidth in FrequencyResponseData since the dcgain method is not implemented there.

I have some other suggestions, but if you agree with the change above then I would start there, since it will move things around.

Coverage: 94.652% (+0.003%) from 94.649% when pulling ab68562 on SCLiao47:feature_bandwidth into c06a205 on python-control:main.

@murrayrm thank you for the comments and suggestions.

The _bandwidth method was implemented as I was following the _dcgain method in the LTI class in lti.py. But as you suggested renaming it to bandwidth can simplify the code structure. One other thing I wasn't so sure about is "what other classes should have the bandwidth method." The FrequencyResponseData class is one of them as you mentioned, and I wonder if there are others as well?

[Regarding development flow]

• I'm still learning the development flow on GitHub. The contribute wiki page (step 11) mentioned that I could keep working on the branch under my own fork. However, does the new commit directly sync to this pull request?
• One other thing I notice while following the contribute wiki page. It seems like GitHub has renamed the master branch to main. Maybe there needs some edition to the wiki page but I'm not so sure.

Hi @SCLiao47

i think only the classes that derive from LTI should get a bandwidth method: tf, ss, and frd.

As for your question about development: adding new commits to your branch is very seamless. Add them to your own branch (on your own computer) and then, when you do git push origin, they will appear right here on your pull request.

Our main branch name is grandfathered in as master.

I've updated the documentation to refer to the main branch (master is deprecated).

@sawyerbfuller @murrayrm thank you for the suggestions and comments. I've restructured the bandwidth method in the LTI class in lti.py.

FRD

As for the FrequencyResponseData class, I wonder how you would suggest implementing the bandwidth method for it? At the moment, the class seems to not support interpolating, which is needed to infer DC gain and compute the bandwidth. However, this will probably fall outside of the scope of this feature. Is there a way to get around this?

Development flow

I think I need to make sure I understand the flow and might need some clarification. In step 8 of the contribute wiki page, the intention is to update and sync the local code (on my machine) to the upstream version (python-control/python-control).

What happened in the instructions:

1. After clicking sync fork and Update branch on github.com, my forked repo (SCLiao47/python-control) will be sync to the upstream version. Both repos are remote on github.
2. The first chunk of command lines (on my machine) is to sync my local main branch to the main of the forked repo on GitHub.
3. The second chunk of command lines is to bring the commits in the local feature branch to the local main. The git rebase allows resolving conflict before merging local feature into upstream main and maintaining a cleaner git graph.

If the above understanding is correct, I wonder about the following:

1. The local main is not intended to be developed and made changed from my understanding. Why do we need to git push after merge origin/main?
2. Can we skip sync fork on github.com and replace git merge origin/main with git merge upstream/main? This will have the same result that the local main will be synced to upstream main, right?

What happened in the instructions:

1. After clicking sync fork and Update branch on github.com, my forked repo (SCLiao47/python-control) will be sync to the upstream version. Both repos are remote on github.

Correct.

2. The first chunk of command lines (on my machine) is to sync my local main branch to the main of the forked repo on GitHub.

Correct.

3. The second chunk of command lines is to bring the commits in the local feature branch to the local main. The git rebase allows resolving conflict before merging local feature into upstream main and maintaining a cleaner git graph.

Yes, the commands make sure that your local feature branch also has all the recent changes of your recently updated local main which should now be at the same commit as upstream/main

If the above understanding is correct, I wonder about the following:

1. The local main is not intended to be developed and made changed from my understanding. Why do we need to git push after merge origin/main?

If you didn't change anything in your local main before the merge, you don't need to push. The push is a noop then. In that case the merge will be a fast-forward and it is sufficient to alternatively just.

git switch main; git pull

But if you have changes in local main (which you should not), you will notice here.

2. Can we skip sync fork on github.com and replace git merge origin/main with git merge upstream/main? This will have the same result that the local main will be synced to upstream main, right?

Yes, but then you need the push so that your origin/main does not stay behind. These sequences are equivalent, assuming origin/main is already the tracked remote branch of your local clone.

1. git switch main; git fetch --all; git merge upstream/main; git push
   

2. on main press "sync fork", then:

   git switch main; git pull