Conversation
Appearantly TG decided to change the size of a send image (again)
|
Review wanted. |
jsmnbom
left a comment
There was a problem hiding this comment.
Looks really good!
Some small quick notes :)
Then we need to decide if we're gonna release it like this or if we need to get sending new files in groups to work first.
telegram/inputmediaphoto.py
Outdated
| if isinstance(media, PhotoSize): | ||
| self.media = media.file_id | ||
| elif hasattr(media, 'read'): | ||
| raise ValueError('We only support file_id or url as a valid media. Sending files is ' |
| @@ -0,0 +1,31 @@ | |||
| #!/usr/bin/env python | |||
There was a problem hiding this comment.
Should these files maybe be in files/?
tests/test_inputmedia.py
Outdated
|
|
||
| def test_with_video(self, video): | ||
| # fixture found in test_video | ||
| imv = InputMediaVideo(video, caption="test 3") |
There was a problem hiding this comment.
I don't think we use abbreviations in the other tests
There was a problem hiding this comment.
We don;t and I'm lazy, will fix
tests/test_inputmedia.py
Outdated
| assert isinstance(messages, list) | ||
| assert len(messages) == 2 | ||
| assert all([isinstance(mes, Message) for mes in messages]) | ||
| assert all([mes.media_group_id == messages[0].media_group_id for mes in messages]) No newline at end of file |
There was a problem hiding this comment.
Maybe a TODO: about needing to test with new files.
tests/test_official.py
Outdated
| field = 'from_user' | ||
| elif name.startswith('InlineQueryResult') and field == 'type': | ||
| continue | ||
| elif name.startswith('InputMedia') and field == 'type': |
There was a problem hiding this comment.
Could you combine with above?
elif (name.startswith('InlineQueryResult') or name.startswith('InputMedia')) and field == 'type':
seems cleaner to me.
There was a problem hiding this comment.
yes, I pondered on it, but I will
| assert photo.width == 1920 | ||
| assert photo.height == 1080 | ||
| assert photo.file_size == 30907 | ||
| assert photo.width == 1280 |
There was a problem hiding this comment.
What changed here?
And how... like it's a smaller image but it takes up more space? :/
There was a problem hiding this comment.
I have no Idea, but it's what came back...
tests/test_official.py
Outdated
| elif name.startswith('InlineQueryResult') and field == 'type': | ||
| continue | ||
| elif name.startswith('InputMedia') and field == 'type': | ||
| elif (name.startswith('InlineQueryResult') or name.startswith('InputMedia')) \ |
There was a problem hiding this comment.
Could you wrap it all in paranthesis instead of using the backslash here maybe?, doesn't look very clean IMO
Make it possible to send an object that will be json-serialized for send_invoice + tests
As discussed do the current
InputMedia*throw aValueErrorwhen supplied with a file instead of afile_idor andurl.This is because the current workings of
InputFileand it's mechanics to encode as multipart form data don't support multiple files. And quite frankly, I have no Idea how to do that.Closes #918