Convert filtering functions to classes #531
Conversation
|
|
||
| logger = logging.getLogger("matchms") | ||
|
|
||
| class AddCompoundName(BaseSpectrumFilter): |
There was a problem hiding this comment.
It might make more sense, to have the Classes in the same file that has the functional equivalent.
There was a problem hiding this comment.
So below the add_compound_name function
There was a problem hiding this comment.
Something that is hard to judge now is if the same functionality is still in the classes that was previously in the function, since we made quite some updates to matchms in the last few weeks. There is a risk that these changes are undone. I am not sure how we could best check this. Anyone any suggestions?
| from matchms.typing import SpectrumType | ||
|
|
||
|
|
||
| class HarmonizeUndefinedTemplate(BaseSpectrumFilter): |
There was a problem hiding this comment.
Maybe good for file organizations to make a separate folder with these kinds of parent classes. The folder could be called FilterTypes for instance.
As per our discussion, Opening the draft PR for discussion.
This implementation is based on the template design pattern, containing a base class that contains two functions process() and abstract method apply_filter(). Then it has further sub base class for filters that share quite a lot of similar code e.g. select_by filter and harmonize_undefined filters. There is not really a change in the current filter function usage to keep usage the same for now, but internally they now use class implementation.