-
Notifications
You must be signed in to change notification settings - Fork 65
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] - Update filter tutorial(s) #232
Conversation
Hi @TomDonoghue, I still have more work to do on this PR, but I made a first attempt at an FIR tutorial. The Widmann 2015 recommendations are in included in the tutorial, so #235 has been squashed into 48a688c. |
I had a quick look at this one. It seems that the math isn't rendering properly? Also, I'm not sure it's particularly needed to get into the details of the math on this. We can't fully cover the technicalities of filters here, and I think more code examples would be better. Also, more general description, introducing, for example what an 'FIR' filter, in particular, is, and the different passtypes (though that might be in a more a different tutorial). Also, in terms of organizing the tutorials, I'm thinking maybe:
|
I'll remove the math - it helped me understand FIR filters and thought it may be useful to others. I agree with the organization. That was the plan I had in mind too. I'm still working on the IIR tutorial but it'll be finished soon. |
@ryanhammonds - if you found the math to be helpful to explain what's up with filters, then maybe we should keep it! I found it hard to get a sense of when it wasn't rendering - but when it prints properly, it might be useful, so feel free to keep it for now! |
Sounds good! The math is fixed by changing |
3cfba19
to
79fe7a8
Compare
Okay - I finally came back around to this one. The stuff you added and organized looks great @ryanhammonds, and I also came around to keeping the math! I kept pushing on it a bit, with the main theme of organizing what's where, and avoiding duplication - so the I split out more stuff from the original tutorial, added a fourth section for "filter checks", and edited from there, trying to edit in more information as I could. Thanks for doing a push on this, it really helped to have a big chunk of this done. This should be ready to merge, I think. @ryanhammonds - when you get a chance, if you could do a quick scan for any typos / bugs / or word quirks, that would be great. If it looks good, we can merge. |
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.
This looks good! I'm pushing commit to correct a few typos I found. One thing we may want to do later is add a filter report section to the end of the "checks" tutorial once #235 is merged.
Awesome, thanks for the check @ryanhammonds - the fixes look great! And yeh, we should definitely update this to reflect the Filter Report once #235 is ready, I'm going to try to get to that soon. Merging this in now! |
This PR updates the filter documentation / tutorials, relating to #229.
@ryanhammonds : following the issue, can you have a go working on this PR? I would suggest digging into the linked papers to guide some extra detail that can be added. If you can organize the new layout, add some detail and extend and you think is needed, I can come back to it after and edit. Thanks!