- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.9k
remove unnecessary to_dict() #834
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
We have some objects that have exactly the same to_dict() method, only specifying that `from_user` should be `from` in the `data`-dict. I refractored this logic to `TelegramObject` and removed those to_dicts() from the code.
| Codecov Report
 @@            Coverage Diff             @@
##           master     #834      +/-   ##
==========================================
- Coverage   91.44%   91.23%   -0.22%     
==========================================
  Files         101      101              
  Lines        4023     4004      -19     
  Branches      614      615       +1     
==========================================
- Hits         3679     3653      -26     
- Misses        200      208       +8     
+ Partials      144      143       -1
 
 Continue to review full report at Codecov. 
 | 
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.
I'm a bit conflicted with this change. On one hand it's a lot less duplicated code which is good most of the time. But on the other hand it's a bit unintuitive to have "from/from_user" be the only thing special thing that's handled in the base. I personally think it makes more sense to have it where it was before, despite the duplicated code.
        
          
                telegram/callbackquery.py
              
                Outdated
          
        
      | bot.edit_message_reply_markup(inline_message_id=update.callback_query.inline_message_id, | ||
| bot.edit_message_reply_markup( | ||
| inline_message_id=update.callback_query.inline_message_id, | 
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.
Please fix the indentation here :)
| @bomjacob I fixed the indentation as you requested. | 
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.
Alright, I can see that, LGTM then :)
We have some objects that have exactly the same to_dict() method, only specifying that
from_usershould befromin thedata-dict. I refractored this logic toTelegramObjectand removed those to_dicts() from the code.