Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions telegram/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,42 @@
MAX_MESSAGES_PER_MINUTE_PER_GROUP = 20
MAX_MESSAGE_ENTITIES = 100
MAX_INLINE_QUERY_RESULTS = 50


class MediaCategory:
Copy link
Member

Choose a reason for hiding this comment

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

Just put these constants in the filters.py, since that's the only place they are used and they aren't meant to be exported to the general user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not necessary if I use built-in mimetypes

"""This are the mime categories of media you can send via telegram.
They are used in `Telegram.ext.filters.Filters.document.`"""
application = "application/"
audio = "audio/"
image = "image/"
video = "video/"
text = "text/"


class Document:
Copy link
Member

Choose a reason for hiding this comment

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

Is there a good reason you're not using the stdlib mimetype?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, just didn't found them. I will fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The stdlib mimetype doesn't work for many mimetypes in telegram. ".exe" documents for example have the mimetype "application/x-ms-dos-executable" if you send it with an official telegram app, but ".exe" is matched to "application/octet-stream" in the stdlib mimetype.

"""These strings are the mime-types of the most common file-types.
They are used in `Telegram.ext.filters.Filters.document.`
Note:
This are only some of the most common mime_types.
There are about 130 different types registered at the moment,
so we didn't add all of them.
Simply use `Filters.document.FileType(mime_type)` to add filters
for other file types"""
apk = "application/vnd.android.package-archive"
docx = "application/vnd.openxmlformats-officedocument.wordprocessingml.document"
exe = "application/x-ms-dos-executable"
gif = "video/mp4"
jpg = "image/jpeg"
mp3 = "audio/mpeg"
pdf = "application/pdf"
png = "image/png"
py = "text/x-python"
svg = "image/svg+xml"
txt = "text/plain"
targz = "application/x-compressed-tar"
wav = "audio/x-wav"
xml = "application/xml"
zip = "application/zip"
113 changes: 111 additions & 2 deletions telegram/ext/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@
# You should have received a copy of the GNU Lesser Public License
# along with this program. If not, see [http://www.gnu.org/licenses/].
"""This module contains the Filters for use with the MessageHandler class."""
from telegram import Chat
import sys

from future.utils import string_types
from telegram import Chat
from telegram import constants


class BaseFilter(object):
Expand Down Expand Up @@ -192,12 +195,118 @@ def filter(self, message):
class _Document(BaseFilter):
name = 'Filters.document'

class Category(BaseFilter):
Copy link
Member

Choose a reason for hiding this comment

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

Please use the convention of lowercasing filters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought I use the PEP 8 Style here, because it's a class to call. It's different to the other filters yet

"""This Filter filters documents by their category in the mime-type attribute

Note:
This Filter only filters by the mime_type of the document,
it doesn't check the validity of the document.
The user can manipulate the mime-type of a message and
send media with wrong types that don't fit to this handler.

Examples:
Filters.documents.Category("audio/") returnes `True` for all types
of audio sent as file, for example "audio/mpeg" or "audio/x-wav"
"""
name = "Filters.document.Category()"
Copy link
Member

Choose a reason for hiding this comment

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

Please do something like
self.name = 'Filters.document.category({})'.format(self.category) in __init__ instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that makes sense. I will fix it.


def __init__(self, category):
"""Initialize the category you want to filter

Args:
category (str, optional): Category of the media you want to filter"""
self.category = category

def filter(self, message):
if message.document:
return bool(message.document.mime_type.startswith(self.category))

application = Category(constants.MediaCategory.application)
Copy link
Member

Choose a reason for hiding this comment

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

Could we perhaps expose these as filters.document.category.application instead of just filters.document.application?

Copy link
Member

Choose a reason for hiding this comment

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

@bomjacob
is this nesting really necessary? I think that filters.document.application is much more easier to use.

audio = Category(constants.MediaCategory.audio)
image = Category(constants.MediaCategory.image)
video = Category(constants.MediaCategory.video)
text = Category(constants.MediaCategory.text)

class FileType(BaseFilter):
Copy link
Member

Choose a reason for hiding this comment

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

Lowercase here too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a class you call directly, and classes should be CamelCase according to PEP8

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, but we ignore PEP8 for filters, since the other filters are simple attributes.

"""This Filter filters documents by their mime-type attribute

Note:
This Filter only filters by the mime_type of the document,
it doesn't check the validity of document.
The user can manipulate the mime-type of a message and
send media with wrong types that don't fit to this handler.

Examples:
Filters.documents.FileType("audio/mpeg") filters all audio in mp3 format.
"""
name = "Filters.document.Filetype()"
Copy link
Member

Choose a reason for hiding this comment

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

Same comment about name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will fix it


def __init__(self, filetype):
"""Initialize the category you want to filter

Args:
filetype (str, optional): mime_type of the media you want to filter"""
self.filetype = filetype

def filter(self, message):
if message.document:
return bool(message.document.mime_type == self.filetype)

apk = FileType(constants.Document.apk)
Copy link
Member

Choose a reason for hiding this comment

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

Could we perhaps expose these as filters.document.filetype.apk instead of just filters.document.apk?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's much more nesting. Not sure we really need this.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, no nesting is okay :)

docx = FileType(constants.Document.docx)
exe = FileType(constants.Document.exe)
gif = FileType(constants.Document.gif)
jpg = FileType(constants.Document.jpg)
mp3 = FileType(constants.Document.mp3)
pdf = FileType(constants.Document.pdf)
py = FileType(constants.Document.py)
svg = FileType(constants.Document.svg)
txt = FileType(constants.Document.txt)
targz = FileType(constants.Document.targz)
wav = FileType(constants.Document.wav)
xml = FileType(constants.Document.xml)
zip = FileType(constants.Document.zip)

def filter(self, message):
return bool(message.document)

document = _Document()
""":obj:`Filter`: Messages that contain :class:`telegram.Document`."""

class FileSize(BaseFilter):
Copy link
Member

Choose a reason for hiding this comment

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

Lowercasing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again PEP8

"""This Filter filters all messages with a `file_size` attribute.
"""
name = "Filters.FileSize()"
Copy link
Member

Choose a reason for hiding this comment

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

Same comment about name


def __init__(self, min=0, max=sys.maxsize):
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a huge fan of using sys.maxsize. Could you instead have it default to None and if it is None just don't compare upper limit?

"""Initialize the limits of the file_size in the `__init__` method.

Args:
min (int, optional): Minimum `file_size` of the message media in Byte.
Default is zero.
max (int, optional): Maximum `file_size` of the message media in Byte.
Default is infinity.
"""
self.min = min
self.max = max

def filter(self, message):
if bool(message.audio):
Copy link
Member

Choose a reason for hiding this comment

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

There's no need for the bool() in these if statements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I will leave it

filesize = message.audio.file_size
elif bool(message.document):
filesize = message.document.file_size
elif bool(message.photo):
filesize = message.photo.file_size
elif bool(message.sticker):
filesize = message.sticker.file_size
elif bool(message.video):
filesize = message.video.file_size
elif bool(message.voice):
filesize = message.voice.file_size
else:
return False
return bool(self.min <= filesize <= self.max)
Copy link
Member

Choose a reason for hiding this comment

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

Also no need for that bool()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will leave it too. Fixed PR will come tomorrow. Thank you for your feedback.


class _Photo(BaseFilter):
name = 'Filters.photo'

Expand Down Expand Up @@ -335,7 +444,7 @@ def filter(self, message):

migrate = _Migrate()
""":obj:`Filter`: Messages that contain :attr:`telegram.Message.migrate_from_chat_id` or
:attr: `telegram.Message.migrate_to_chat_id`."""
:attr: `telegram.Message.migrate_from_chat_id`."""

class _PinnedMessage(BaseFilter):
name = 'Filters.status_update.pinned_message'
Expand Down