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
Improve handling of custom objects in BP.insert/replace_bot #2151
Merged
Merged
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
521ed1a
Handle unpickable objects
Bibo-Joshi b3c2959
Improve coverage
Bibo-Joshi 62480e8
Add user warning
Bibo-Joshi 54841c0
make comparison to REPLACED_BOT safe
Bibo-Joshi 238b712
Merge branch 'master' into handle-unpickable-objects
Bibo-Joshi cc09547
make pre-commit happy
Bibo-Joshi 6b1245e
Shorten warning
Bibo-Joshi File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -17,7 +17,7 @@ | |||||
# 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 BasePersistence class.""" | ||||||
|
||||||
import warnings | ||||||
from abc import ABC, abstractmethod | ||||||
from collections import defaultdict | ||||||
from copy import copy | ||||||
|
@@ -128,12 +128,12 @@ def set_bot(self, bot: Bot) -> None: | |||||
self.bot = bot | ||||||
|
||||||
@classmethod | ||||||
def replace_bot(cls, obj: object) -> object: | ||||||
def replace_bot(cls, obj: object) -> object: # pylint: disable=R0911 | ||||||
""" | ||||||
Replaces all instances of :class:`telegram.Bot` that occur within the passed object with | ||||||
:attr:`REPLACED_BOT`. Currently, this handles objects of type ``list``, ``tuple``, ``set``, | ||||||
``frozenset``, ``dict``, ``defaultdict`` and objects that have a ``__dict__`` or | ||||||
``__slot__`` attribute. | ||||||
``__slot__`` attribute, excluding objects that can't be copied with `copy.copy`. | ||||||
|
||||||
Args: | ||||||
obj (:obj:`object`): The object | ||||||
|
@@ -146,7 +146,18 @@ def replace_bot(cls, obj: object) -> object: | |||||
if isinstance(obj, (list, tuple, set, frozenset)): | ||||||
return obj.__class__(cls.replace_bot(item) for item in obj) | ||||||
|
||||||
new_obj = copy(obj) | ||||||
try: | ||||||
new_obj = copy(obj) | ||||||
except Exception: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
warnings.warn( | ||||||
'BasePersistence.replace_bot caught an error while trying to copy an object. ' | ||||||
'Objects that can not be copied will be assumed to not contain a telegram.Bot ' | ||||||
'instance and will not be handled further. See the docs of ' | ||||||
'BasePersistence.replace_bot for more information.', | ||||||
RuntimeWarning, | ||||||
Bibo-Joshi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
) | ||||||
return obj | ||||||
|
||||||
if isinstance(obj, (dict, defaultdict)): | ||||||
new_obj = cast(dict, new_obj) | ||||||
new_obj.clear() | ||||||
|
@@ -173,7 +184,7 @@ def insert_bot(self, obj: object) -> object: # pylint: disable=R0911 | |||||
Replaces all instances of :attr:`REPLACED_BOT` that occur within the passed object with | ||||||
:attr:`bot`. Currently, this handles objects of type ``list``, ``tuple``, ``set``, | ||||||
``frozenset``, ``dict``, ``defaultdict`` and objects that have a ``__dict__`` or | ||||||
``__slot__`` attribute. | ||||||
``__slot__`` attribute, excluding objects that can't be copied with `copy.copy`. | ||||||
|
||||||
Args: | ||||||
obj (:obj:`object`): The object | ||||||
|
@@ -183,12 +194,23 @@ def insert_bot(self, obj: object) -> object: # pylint: disable=R0911 | |||||
""" | ||||||
if isinstance(obj, Bot): | ||||||
return self.bot | ||||||
if obj == self.REPLACED_BOT: | ||||||
if isinstance(obj, str) and obj == self.REPLACED_BOT: | ||||||
return self.bot | ||||||
if isinstance(obj, (list, tuple, set, frozenset)): | ||||||
return obj.__class__(self.insert_bot(item) for item in obj) | ||||||
|
||||||
new_obj = copy(obj) | ||||||
try: | ||||||
new_obj = copy(obj) | ||||||
except Exception: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
warnings.warn( | ||||||
'BasePersistence.insert_bot caught an error while trying to copy an object. ' | ||||||
'Objects that can not be copied will be assumed to not contain a telegram.Bot ' | ||||||
'instance and will not be handled further. See the docs of ' | ||||||
'BasePersistence.insert_bot for more information.', | ||||||
RuntimeWarning, | ||||||
) | ||||||
Bibo-Joshi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
return obj | ||||||
|
||||||
if isinstance(obj, (dict, defaultdict)): | ||||||
new_obj = cast(dict, new_obj) | ||||||
new_obj.clear() | ||||||
|
@@ -207,6 +229,7 @@ def insert_bot(self, obj: object) -> object: # pylint: disable=R0911 | |||||
self.insert_bot(self.insert_bot(getattr(new_obj, attr_name))), | ||||||
) | ||||||
return new_obj | ||||||
|
||||||
return obj | ||||||
|
||||||
@abstractmethod | ||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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.
pylint complains about the methods having too many return statements, so moving that line to the
except Exception
will make the test fail …