The Wayback Machine - https://web.archive.org/web/20211026015912/https://github.com/TheAlgorithms/Python/pull/5230
Skip to content
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

Jump to bottom

Simple audio filters #5230

Merged
merged 18 commits into from Oct 23, 2021
Merged

Simple audio filters #5230

merged 18 commits into from Oct 23, 2021
+403 −0

Conversation

@martmists-gh
Copy link
Contributor

@martmists-gh martmists-gh commented Oct 11, 2021

Describe your change:

Add N-order IIR filter and Butterworth coefficients calculations

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.
@martmists-gh
Add IIR Filter and Butterworth design functions
2dcb414 
Signed-off-by: Martmists <[email protected]>
@martmists-gh
naming conventions and missing type hints
01574c8 
Signed-off-by: Martmists <[email protected]>
@martmists-gh
Link wikipedia in IIRFilter
9770407 
Signed-off-by: Martmists <[email protected]>
algorithms-keeper[bot]
algorithms-keeper bot reviewed Oct 11, 2021
View changes
Copy link

@algorithms-keeper algorithms-keeper bot left a comment

Click here to look at the relevant links ⬇️

🔗 Relevant Links

Repository:

Python:

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:

  • @algorithms-keeper review to trigger the checks for only added pull request files
  • @algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.

NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
audio_filters/butterworth_filter.py Outdated Show resolved Hide resolved
audio_filters/butterworth_filter.py Outdated Show resolved Hide resolved
audio_filters/butterworth_filter.py Outdated Show resolved Hide resolved
audio_filters/butterworth_filter.py Outdated Show resolved Hide resolved
audio_filters/butterworth_filter.py Outdated Show resolved Hide resolved
audio_filters/iir_filter.py Outdated Show resolved Hide resolved
audio_filters/iir_filter.py Outdated Show resolved Hide resolved
audio_filters/iir_filter.py Show resolved Hide resolved
audio_filters/show_response.py Outdated Show resolved Hide resolved
audio_filters/show_response.py Outdated Show resolved Hide resolved
@martmists-gh
Add doctests and None return types
adc612d 
Signed-off-by: Martmists <[email protected]>
algorithms-keeper[bot]
algorithms-keeper bot reviewed Oct 11, 2021
View changes
Copy link

@algorithms-keeper algorithms-keeper bot left a comment

Click here to look at the relevant links ⬇️

🔗 Relevant Links

Repository:

Python:

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:

  • @algorithms-keeper review to trigger the checks for only added pull request files
  • @algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.

NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
audio_filters/show_response.py Show resolved Hide resolved
audio_filters/show_response.py Show resolved Hide resolved
audio_filters/show_response.py Show resolved Hide resolved
@martmists-gh
More doctests
ce8bf6d 
Signed-off-by: Martmists <[email protected]>
@martmists-gh
Copy link
Contributor Author

@martmists-gh martmists-gh commented Oct 11, 2021

Note: doctests may fail due to typing.Protocol requiring doctests, see also #5231.
cclauss
cclauss reviewed Oct 11, 2021
View changes
audio_filters/butterworth_filter.py Outdated Show resolved Hide resolved
cclauss
cclauss requested changes Oct 11, 2021
View changes
audio_filters/show_response.py Outdated Show resolved Hide resolved
@martmists-gh
Requested changes
92e58da 
Signed-off-by: Martmists <[email protected]>
@martmists-gh
run pre-commit
4fe509d 
Signed-off-by: Martmists <[email protected]>
@martmists-gh
Make mypy stop complaining about ints vs floats
3f71475 
Signed-off-by: Martmists <[email protected]>
cclauss
cclauss reviewed Oct 15, 2021
View changes
audio_filters/show_response.py Outdated Show resolved Hide resolved
cclauss
cclauss reviewed Oct 15, 2021
View changes
audio_filters/show_response.py Outdated Show resolved Hide resolved
@martmists-gh
Use slower listcomp to make it more readable
6a69c43 
Signed-off-by: Martmists <[email protected]>
@martmists-gh
Make doctests happy
72ab283 
Signed-off-by: Martmists <[email protected]>
@cclauss
Copy link
Member

@cclauss cclauss commented Oct 15, 2021

Is there proof that list comprehensions are really slower?!? Perhaps a timeit benchmark?

According to CONTRIBUTING.md, algorithmic functions should return values and should not print or plot. Is there a way to separate the calculations into separate functions from plotting? In this way, we can put doctests on the algorithmic functions.
cclauss
cclauss reviewed Oct 15, 2021
View changes
requirements.txt Outdated Show resolved Hide resolved
@martmists-gh
Remove scipy
fb839f5 
Signed-off-by: Martmists <[email protected]>
@martmists-gh martmists-gh requested a review from cclauss Oct 15, 2021
@martmists-gh
Test coefficients from bw filters
05e8e1b 
Signed-off-by: Martmists <[email protected]>
@martmists-gh
Copy link
Contributor Author

@martmists-gh martmists-gh commented Oct 22, 2021

@cclauss any updates on this?
cclauss
cclauss reviewed Oct 23, 2021
View changes
audio_filters/show_response.py Outdated Show resolved Hide resolved
martmists-gh and others added 2 commits Oct 23, 2021
@martmists-gh @cclauss
Protocol test
0fdd49e 
Co-authored-by: Christian Clauss <[email protected]>
@martmists-gh
Make requested change
dedc0d5 
Signed-off-by: Martmists <[email protected]>
algorithms-keeper[bot]
algorithms-keeper bot reviewed Oct 23, 2021
View changes
Copy link

@algorithms-keeper algorithms-keeper bot left a comment

Click here to look at the relevant links ⬇️

🔗 Relevant Links

Repository:

Python:

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:

  • @algorithms-keeper review to trigger the checks for only added pull request files
  • @algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.

NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
audio_filters/show_response.py Outdated Show resolved Hide resolved
@martmists-gh
Types
ef2b952 
Signed-off-by: Martmists <[email protected]>
cclauss
cclauss approved these changes Oct 23, 2021
View changes
audio_filters/show_response.py Outdated Show resolved Hide resolved
cclauss
cclauss approved these changes Oct 23, 2021
View changes
audio_filters/show_response.py Outdated Show resolved Hide resolved
audio_filters/iir_filter.py Outdated Show resolved Hide resolved
audio_filters/iir_filter.py Outdated Show resolved Hide resolved
audio_filters/show_response.py Show resolved Hide resolved
audio_filters/show_response.py Outdated Show resolved Hide resolved
@cclauss
Apply suggestions from code review
9848caf
cclauss
cclauss approved these changes Oct 23, 2021
View changes
audio_filters/iir_filter.py Outdated Show resolved Hide resolved
audio_filters/show_response.py Outdated Show resolved Hide resolved
@cclauss
Apply suggestions from code review
2d9ccc4
@cclauss
Update butterworth_filter.py
6cb4e2c
@cclauss
Copy link
Member

@cclauss cclauss commented Oct 23, 2021

@martmists-gh Thanks for your super cool contribution!!! I am OK with this version.

If it is good for you, please let me know and I will merge it.
@martmists-gh
Copy link
Contributor Author

@martmists-gh martmists-gh commented Oct 23, 2021

LGTM
@cclauss cclauss merged commit 00a6701 into TheAlgorithms:master Oct 23, 2021
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants
@martmists-gh @cclauss