-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DOC: Add See Also links and Example graphs to filter design *ord functions #3234
Conversation
illustrating how the passband and stopband specs constrain the filter frequency response
See Also | ||
-------- | ||
ellip : Filter design using order and critical points | ||
buttord, cheb1ord, cheb2ord, : Find order and critical points |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the comma after cheb2ord
, otherwise the doc build crashes
For the rest this looks good to me. Nice figures. |
Ok, fixed both of those. |
Add See Also links and Example graphs to filter design *ord functions
Thanks, merging |
It looks like I screwed something up. The images show up on http://docs.scipy.org/doc/scipy-dev/reference/generated/scipy.signal.ellipord.html and http://docs.scipy.org/doc/scipy-dev/reference/generated/scipy.signal.buttord.html, but not on http://docs.scipy.org/doc/scipy-dev/reference/generated/scipy.signal.cheb1ord.html and http://docs.scipy.org/doc/scipy-dev/reference/generated/scipy.signal.cheb2ord.html Any idea why? Also the lines like
are not working. I guess they can't have commas in them? I could reword it to
also should have added an example/graph for |
If you just paste the example into a clean IPython shell you'll see where it fails.The issue should be that you're using |
As for See Also, I think it's either a list of functions or a single function with a description behind it. |
I tried to add the fixes to the same patch, but they're not showing up here, maybe because it's already been merged? They are visible here: endolith@137ace8 |
You indeed can't add commits to a PR that has been merged already. I'll cherry-pick and push that commit (after double checking it). |
Pushed doc fix commit in 28f9d10 |
Also fixed a typo in a printed warning. Should that be converted to a raised warning or exception?