Simple audio filters by Martmists-GH · Pull Request #5230 · TheAlgorithms/Python · GitHub
Skip to content

Simple audio filters - #5230

Merged
cclauss merged 18 commits into
TheAlgorithms:masterfrom
Martmists-GH:master
Oct 23, 2021
Merged

Simple audio filters#5230
cclauss merged 18 commits into
TheAlgorithms:masterfrom
Martmists-GH:master

Conversation

@Martmists-GH

@Martmists-GH Martmists-GH commented Oct 11, 2021

Copy link
Copy Markdown
Contributor

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 <martmists@gmail.com>
@Martmists-GH
naming conventions and missing type hints
01574c8 
Signed-off-by: Martmists <martmists@gmail.com>
@Martmists-GH
Link wikipedia in IIRFilter
9770407 
Signed-off-by: Martmists <martmists@gmail.com>
@ghost ghost added awaiting reviews This PR is ready to be reviewed require tests Tests [doctest/unittest/pytest] are required require type hints https://docs.python.org/3/library/typing.html labels Oct 11, 2021
ghost
ghost reviewed Oct 11, 2021
View reviewed changes

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread audio_filters/butterworth_filter.py Outdated
Comment thread audio_filters/butterworth_filter.py Outdated
Comment thread audio_filters/butterworth_filter.py Outdated
Comment thread audio_filters/butterworth_filter.py Outdated
Comment thread audio_filters/butterworth_filter.py Outdated
Comment thread audio_filters/iir_filter.py Outdated
Comment thread audio_filters/iir_filter.py Outdated
Comment thread audio_filters/iir_filter.py
Comment thread audio_filters/show_response.py Outdated
Comment thread audio_filters/show_response.py Outdated
@Martmists-GH
Add doctests and None return types
adc612d 
Signed-off-by: Martmists <martmists@gmail.com>
@ghost ghost removed the require type hints https://docs.python.org/3/library/typing.html label Oct 11, 2021
ghost
ghost reviewed Oct 11, 2021
View reviewed changes

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread audio_filters/show_response.py
Comment thread audio_filters/show_response.py
Comment thread audio_filters/show_response.py
@Martmists-GH
More doctests
ce8bf6d 
Signed-off-by: Martmists <martmists@gmail.com>
@ghost ghost removed the require tests Tests [doctest/unittest/pytest] are required label Oct 11, 2021
@Martmists-GH

Martmists-GH commented Oct 11, 2021

Copy link
Copy Markdown
Contributor Author

cclauss
cclauss reviewed Oct 11, 2021
View reviewed changes
Comment thread audio_filters/butterworth_filter.py Outdated
cclauss
cclauss requested changes Oct 11, 2021
View reviewed changes
Comment thread audio_filters/show_response.py Outdated
@ghost ghost added awaiting changes A maintainer has requested changes to this PR tests are failing Do not merge until tests pass and removed awaiting reviews This PR is ready to be reviewed labels Oct 11, 2021
@Martmists-GH
Requested changes
92e58da 
Signed-off-by: Martmists <martmists@gmail.com>
@ghost ghost added awaiting reviews This PR is ready to be reviewed and removed awaiting changes A maintainer has requested changes to this PR labels Oct 11, 2021
@Martmists-GH
run pre-commit
4fe509d 
Signed-off-by: Martmists <martmists@gmail.com>
@Martmists-GH
Make mypy stop complaining about ints vs floats
3f71475 
Signed-off-by: Martmists <martmists@gmail.com>
cclauss
cclauss reviewed Oct 15, 2021
View reviewed changes
Comment thread audio_filters/show_response.py Outdated
cclauss
cclauss reviewed Oct 15, 2021
View reviewed changes
Comment thread audio_filters/show_response.py Outdated
@Martmists-GH
Use slower listcomp to make it more readable
6a69c43 
Signed-off-by: Martmists <martmists@gmail.com>
@Martmists-GH
Make doctests happy
72ab283 
Signed-off-by: Martmists <martmists@gmail.com>
@cclauss

cclauss commented Oct 15, 2021

Copy link
Copy Markdown
Member

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 reviewed changes
Comment thread audio_filters/butterworth_filter.py
cclauss
cclauss reviewed Oct 15, 2021
View reviewed changes
Comment thread requirements.txt Outdated
@Martmists-GH
Remove scipy
fb839f5 
Signed-off-by: Martmists <martmists@gmail.com>
@Martmists-GH
Martmists-GH requested a review from cclauss October 15, 2021 11:19
@Martmists-GH
Test coefficients from bw filters
05e8e1b 
Signed-off-by: Martmists <martmists@gmail.com>
@Martmists-GH

Martmists-GH commented Oct 22, 2021

Copy link
Copy Markdown
Contributor Author

@cclauss any updates on this?

cclauss
cclauss reviewed Oct 23, 2021
View reviewed changes
Comment thread audio_filters/show_response.py Outdated
cclauss
cclauss reviewed Oct 23, 2021
View reviewed changes
Comment thread audio_filters/show_response.py
Martmists-GH and others added 2 commits October 23, 2021 18:15
@Martmists-GH @cclauss
Protocol test
0fdd49e 
Co-authored-by: Christian Clauss <cclauss@me.com>
@Martmists-GH
Make requested change
dedc0d5 
Signed-off-by: Martmists <martmists@gmail.com>
@ghost ghost added the require type hints https://docs.python.org/3/library/typing.html label Oct 23, 2021
ghost
ghost reviewed Oct 23, 2021
View reviewed changes

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread audio_filters/show_response.py Outdated
@Martmists-GH
Types
ef2b952 
Signed-off-by: Martmists <martmists@gmail.com>
@ghost ghost removed the require type hints https://docs.python.org/3/library/typing.html label Oct 23, 2021
cclauss
cclauss approved these changes Oct 23, 2021
View reviewed changes
Comment thread audio_filters/show_response.py Outdated
@ghost ghost removed the awaiting reviews This PR is ready to be reviewed label Oct 23, 2021
cclauss
cclauss approved these changes Oct 23, 2021
View reviewed changes
Comment thread audio_filters/show_response.py Outdated
Comment thread audio_filters/iir_filter.py Outdated
Comment thread audio_filters/iir_filter.py Outdated
Comment thread audio_filters/show_response.py
Comment thread audio_filters/show_response.py Outdated
@ghost ghost added the awaiting reviews This PR is ready to be reviewed label Oct 23, 2021
cclauss
cclauss approved these changes Oct 23, 2021
View reviewed changes
Comment thread audio_filters/iir_filter.py Outdated
Comment thread audio_filters/show_response.py Outdated
@ghost ghost removed the awaiting reviews This PR is ready to be reviewed label Oct 23, 2021
@ghost ghost added the awaiting reviews This PR is ready to be reviewed label Oct 23, 2021
@ghost ghost removed the tests are failing Do not merge until tests pass label Oct 23, 2021
@cclauss

cclauss commented Oct 23, 2021

Copy link
Copy Markdown
Member

@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

Martmists-GH commented Oct 23, 2021

Copy link
Copy Markdown
Contributor Author

@cclauss
cclauss merged commit 00a6701 into TheAlgorithms:master Oct 23, 2021
@ghost ghost removed the awaiting reviews This PR is ready to be reviewed label Oct 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cclauss cclauss cclauss approved these changes
+1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Martmists-GH @cclauss