Custom filters #4
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zatkins2
requested changes
Feb 21, 2024
There was a problem hiding this comment.
Did you consider making a separate filter for this new feature? That is probably simpler than modifying each of the other filters to accommodate this new feature. You would only need to write a new model function (registered with model=True). You wouldn't need to write a new simulation filter, you would only need to add a registration on top of existing filters (e.g., analogous to how ivar_basic and raw_ivar_basic share a simulation filters). Maybe there is a downside to this but I think it would be more modular/easier to maintain.
If you prefer to modify the existing filters, I think you missed adding your new feature to iso_harmonic_ivar_scaledep_model
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added the option to filter a map by user-provided harmonic filter. Useful in cases where you would not want to filter by the the pseudo-spectrum of the data. Also added missing lim and lim0 arguments for the wavelet noise model.