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

Make filters and/or-able using bitwise operators. #411

Merged
merged 14 commits into from Oct 19, 2016
Merged

Conversation

@jsmnbom
Copy link
Member

@jsmnbom jsmnbom commented Sep 14, 2016

Would allow filters to be and'ed/or'ed.
Using syntax like this:

handler = MessageHandler((Filters.text | Filters.venue) & Filters.forwarded, callback)

Currently this isn't too useful since the only filter that can be and'ed to any other is forwarded, but that should all change with #409.

The code currently requires all filters to be created as a class and then initiated before they can be used, which could be an inconvenience to the user if they want to create their own filters. To remedy this we could add a decorator like this:

def Filter(f):
    def wrapper(self, message):
        return f(message)
    return type(f.__name__, (BaseFilter,), {'__call__': wrapper})()

That would allow users (and us) to make filters like this:

@Filter
def Text(self, message):
    return bool(message.text and not message.text.startswith('/'))

The reason I didn't immediately go with this, is that it might be confusing if someone (especially someone new to python) reading the code stumbles upon a dynamic class creator.
Thoughts?

  • Figure out implementation
  • Write the code
  • Write the docs
  • Add snippet to wiki
  • Wait for #409 to get merged?

Edit: After talking in dev group, the decorator has been declared too gross to look at :P

See associated PR for more info.
@jh0ker jh0ker added this to the 5.1 milestone Sep 20, 2016
@jh0ker jh0ker removed this from the 5.1 milestone Sep 20, 2016
@jh0ker jh0ker mentioned this pull request Sep 20, 2016
jsmnbom added 5 commits Sep 24, 2016
__call__ is scary looking for users wanted to create their own filters.
Also allows us to put additional logic in __call__ if we want in the future.
# Conflicts:
#	telegram/ext/messagehandler.py
#	tests/test_filters.py
Also add tests with both & and |.
Also deprecates actually using a list.
Copy link
Member

@jh0ker jh0ker left a comment

Just a few nitpic documentation changes, otherwise LGTM 👍

And:
>>> (Filters.text & Filters.entity(MENTION)
Copy link
Member

@jh0ker jh0ker Oct 10, 2016

missed a ) here

Also works with more than two filters:
>>> (Filters.text & (Filters.entity(URL |Filters.entity(TEXT_LINK))))
Copy link
Member

@jh0ker jh0ker Oct 10, 2016

this also looks wrong to me

class MergedFilter(BaseFilter):
"""Represents a filter consisting of two other filters."""

def __init__(self, base_filter, and_filter=None, or_filter=None):
Copy link
Member

@jh0ker jh0ker Oct 10, 2016

Please add a docstring indicating that and_filter and or_filter are mutually exclusive

@jsmnbom jsmnbom dismissed jh0ker’s stale review Oct 15, 2016

Changes have been adressed.

@jsmnbom jsmnbom merged commit 4e5f458 into master Oct 19, 2016
3 checks passed
@jsmnbom jsmnbom deleted the bitwise-filters branch May 20, 2017
@Eldinnie Eldinnie mentioned this pull request May 5, 2018
@github-actions github-actions bot locked and limited conversation to collaborators Aug 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants