-
Notifications
You must be signed in to change notification settings - Fork 5.9k
Fixed version to PR #880 #889
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #889 +/- ##
==========================================
- Coverage 91.7% 91.55% -0.15%
==========================================
Files 101 101
Lines 4063 4123 +60
Branches 623 635 +12
==========================================
+ Hits 3726 3775 +49
- Misses 198 202 +4
- Partials 139 146 +7
Continue to review full report at Codecov.
|
| filesize = message.audio.file_size | ||
| elif message.document: | ||
| filesize = message.document.file_size | ||
| elif message.photo: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
message.photo is an array of different photo sizes. Not sure if the filter even makes here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, thanks. Didn't thought about that. What about just using the photo in the highest resolution?
| elif message.document: | ||
| filesize = message.document.file_size | ||
| elif message.photo: | ||
| filesize = message.photo.file_size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this line will fail
|
Ideas for |
| """This Filter filters documents by their category in the mime-type attribute | ||
| Note: | ||
| This Filter only filters by the mime_type of the document, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You didn't put an Args: section, and you should specifically mention that mime_type.startswith is used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see you put the Args: in the __init__, sorry
| of audio sent as file, for example "audio/mpeg" or "audio/x-wav" | ||
| """ | ||
|
|
||
| def __init__(self, category): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps it would be clearer if we renamed the category parameter to mime_type_prefix
| Filters.documents.file_type("audio/mpeg") filters all audio in mp3 format. | ||
| """ | ||
|
|
||
| def __init__(self, filetype): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and filetype to mime_type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, makes sense
| video = category("video/") | ||
| text = category("text/") | ||
|
|
||
| class file_type(BaseFilter): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and perhaps even name the filter mime_type, to avoid confusion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will change
|
So after some discussion with @code1mountain, we came to the conclusion it would make the most sense to allow any message where at least one of the different Then in a second PR, we add helper methods to Opinions? |
|
What's the status on this? |
|
closing in favor of #1046 |
Changes to PR #880 :
Added tests
Renamed Category Filter to category
Renamed FileSize Filter to file_size
Renamed FileType Filter to file_type
Removed mime_types from constants
Standard values at file_size Filter are None now and are only compared if a value is given -> no sys.maxsize
Usage of stdlib mimetypes, but sometimes types are missing or different from the mime_types sent by the official telegram app. Hardcoded the strings in this cases.
Changed the name attribute of category, file_size and file_type, so their attributes are shown
Still solves Issue #875