Skip to content

Conversation

@jatinkhilnani
Copy link
Contributor

@jatinkhilnani jatinkhilnani commented Jul 28, 2020

Low-, band- and high-pass filter deformer (#1)

@bmcfee
Copy link
Owner

bmcfee commented Jul 28, 2020

@jatinkhilnani I didn't realize you were still working on this. Another student has also been working on this deformer, though her implementation may be pretty different from yours. Perhaps there's a way to combine them?

@jatinkhilnani
Copy link
Contributor Author

jatinkhilnani commented Jul 28, 2020

Continued on it once PR #80 was merged, to include any further feedback (if any). Should have checked with you first.

Based on the PR #83, the implementation seems more exhaustive as I was not sure of the annotation impact. Two items that I see have to be considered though.

  1. Handling of the cutoff parameter can be made concise and integrated
  2. test_deformers.py has conflict since it does not have modifications from PR Clipping deformer & cosmetic fixes #80

Additional points.
3. I think the three methods for RandomFilter can be merged in one
4. LinearFilter implementation can be taken from this PR

There are benefits of combining the two implementations, should I coordinate with the author then? Please suggest, thanks!

@jatinkhilnani
Copy link
Contributor Author

@bmcfee Should I proceed by taking code from PR #83 and making the suggested modifications along with LinearFilter implementation, or suggest otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants