Conversation
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Looks good overall IMO :) Was able to go right into nitpicking ;P
This was done since Bot's init doesn't accept **kwargs so custom deepcopying/pickle would fail.
Also some code cleanup
provides speed benefits for py 3.8+ users
Attributes won't be assigned to the new object otherwise
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Thanks for the work on this! I left a number of comments.
Additionally, please elaborate in the docstring of Bot why bots should not be serialized and that for that reason pickling will raise an exception
|
Current approach of Also fails when there are extra args and the constructor does not accept **kwargs. |
|
Another way to achieve deepcopying without actually defining import copy
from telegram.ext._utils.stack import was_called_by
...
if was_called_by(inspect.currentframe(), Path(copy.__file__)):
return self._get_attrs(include_private=True, recursive=False)
return self._get_attrs(include_private=True, recursive=False, remove_bot=True)but didn't use this since it feels a bit hacky and unpythonic, although its elegant. |
| return self._bot | ||
| if pid == _REPLACED_UNKNOWN_BOT: | ||
| return None | ||
| raise pickle.UnpicklingError("Found unknown persistent id when unpickling!") |
There was a problem hiding this comment.
codecov gonna complain about this since I don't think it's possible to test this...
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Finally get's a LGTM from me 😃 Thanks for the effort you put into this! Is there anything left from your side? Otherwise we can merge.
|
@Bibo-Joshi Nope, all done and am happy with it now |
Closes #2882
Checklist for PRs
.. versionadded:: version,.. versionchanged:: versionor.. deprecated:: versionto the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)AUTHORS.rst(optional)Breaking changes:
insert/replace_botwas removed so user may have to useset_bot(bot)before using shortcuts.