Skip to content

Use @abstractmethod instead of rasing NotImplementedError #1905

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 1, 2020
Merged

Use @abstractmethod instead of rasing NotImplementedError #1905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions telegram/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,10 @@
except ImportError:
import json

from abc import ABCMeta


class TelegramObject(object):
"""Base class for most telegram objects."""

__metaclass__ = ABCMeta
_id_attrs = ()

def __str__(self):
Expand Down
20 changes: 11 additions & 9 deletions telegram/ext/basepersistence.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
# along with this program. If not, see [http://www.gnu.org/licenses/].
"""This module contains the BasePersistence class."""

from abc import ABC, abstractmethod

class BasePersistence(object):

class BasePersistence(ABC):
"""Interface class for adding persistence to your bot.
Subclass this object for different implementations of a persistent bot.

Expand Down Expand Up @@ -57,6 +59,7 @@ def __init__(self, store_user_data=True, store_chat_data=True, store_bot_data=Tr
self.store_chat_data = store_chat_data
self.store_bot_data = store_bot_data

@abstractmethod
def get_user_data(self):
""""Will be called by :class:`telegram.ext.Dispatcher` upon creation with a
persistence object. It should return the user_data if stored, or an empty
Expand All @@ -65,8 +68,8 @@ def get_user_data(self):
Returns:
:obj:`defaultdict`: The restored user data.
"""
raise NotImplementedError

@abstractmethod
def get_chat_data(self):
""""Will be called by :class:`telegram.ext.Dispatcher` upon creation with a
persistence object. It should return the chat_data if stored, or an empty
Expand All @@ -75,8 +78,8 @@ def get_chat_data(self):
Returns:
:obj:`defaultdict`: The restored chat data.
"""
raise NotImplementedError

@abstractmethod
def get_bot_data(self):
""""Will be called by :class:`telegram.ext.Dispatcher` upon creation with a
persistence object. It should return the bot_data if stored, or an empty
Expand All @@ -85,8 +88,8 @@ def get_bot_data(self):
Returns:
:obj:`defaultdict`: The restored bot data.
"""
raise NotImplementedError

@abstractmethod
def get_conversations(self, name):
""""Will be called by :class:`telegram.ext.Dispatcher` when a
:class:`telegram.ext.ConversationHandler` is added if
Expand All @@ -99,8 +102,8 @@ def get_conversations(self, name):
Returns:
:obj:`dict`: The restored conversations for the handler.
"""
raise NotImplementedError

@abstractmethod
def update_conversation(self, name, key, new_state):
"""Will be called when a :attr:`telegram.ext.ConversationHandler.update_state`
is called. this allows the storeage of the new state in the persistence.
Expand All @@ -110,8 +113,8 @@ def update_conversation(self, name, key, new_state):
key (:obj:`tuple`): The key the state is changed for.
new_state (:obj:`tuple` | :obj:`any`): The new state for the given key.
"""
raise NotImplementedError

@abstractmethod
def update_user_data(self, user_id, data):
"""Will be called by the :class:`telegram.ext.Dispatcher` after a handler has
handled an update.
Expand All @@ -120,8 +123,8 @@ def update_user_data(self, user_id, data):
user_id (:obj:`int`): The user the data might have been changed for.
data (:obj:`dict`): The :attr:`telegram.ext.dispatcher.user_data` [user_id].
"""
raise NotImplementedError

@abstractmethod
def update_chat_data(self, chat_id, data):
"""Will be called by the :class:`telegram.ext.Dispatcher` after a handler has
handled an update.
Expand All @@ -130,16 +133,15 @@ def update_chat_data(self, chat_id, data):
chat_id (:obj:`int`): The chat the data might have been changed for.
data (:obj:`dict`): The :attr:`telegram.ext.dispatcher.chat_data` [chat_id].
"""
raise NotImplementedError

@abstractmethod
def update_bot_data(self, data):
"""Will be called by the :class:`telegram.ext.Dispatcher` after a handler has
handled an update.

Args:
data (:obj:`dict`): The :attr:`telegram.ext.dispatcher.bot_data` .
"""
raise NotImplementedError

def flush(self):
"""Will be called by :class:`telegram.ext.Updater` upon receiving a stop signal. Gives the
Expand Down
6 changes: 3 additions & 3 deletions telegram/ext/filters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@
import re

from future.utils import string_types
from abc import ABC, abstractmethod

from telegram import Chat, Update, MessageEntity

__all__ = ['Filters', 'BaseFilter', 'InvertedFilter', 'MergedFilter']


class BaseFilter(object):
class BaseFilter(ABC):
"""Base class for all Message Filters.

Subclassing from this class filters to be combined using bitwise operators:
Expand Down Expand Up @@ -103,6 +104,7 @@ def __repr__(self):
self.name = self.__class__.__name__
return self.name

@abstractmethod
def filter(self, update):
"""This method must be overwritten.

Expand All @@ -118,8 +120,6 @@ def filter(self, update):

"""

raise NotImplementedError


class InvertedFilter(BaseFilter):
"""Represents a filter that has been inverted.
Expand Down
6 changes: 4 additions & 2 deletions telegram/ext/handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
# along with this program. If not, see [http://www.gnu.org/licenses/].
"""This module contains the base class for handlers as used by the Dispatcher."""

from abc import ABC, abstractmethod

class Handler(object):

class Handler(ABC):
"""The base class for all update handlers. Create custom handlers by inheriting from it.

Attributes:
Expand Down Expand Up @@ -82,6 +84,7 @@ def __init__(self,
self.pass_user_data = pass_user_data
self.pass_chat_data = pass_chat_data

@abstractmethod
def check_update(self, update):
"""
This method is called to determine if an update should be handled by
Expand All @@ -96,7 +99,6 @@ def check_update(self, update):
when the update gets handled.

"""
raise NotImplementedError

def handle_update(self, update, dispatcher, check_result, context=None):
"""
Expand Down
36 changes: 36 additions & 0 deletions tests/test_dispatcher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,12 @@ def get_user_data(self):
def update_user_data(self, user_id, data):
raise Exception

def get_conversations(self, name):
pass

def update_conversation(self, name, key, new_state):
pass

def start1(b, u):
pass

Expand Down Expand Up @@ -470,6 +476,21 @@ def update_chat_data(self, chat_id, data):
def update_user_data(self, user_id, data):
self.update(data)

def get_chat_data(self):
pass

def get_bot_data(self):
pass

def get_user_data(self):
pass

def get_conversations(self, name):
pass

def update_conversation(self, name, key, new_state):
pass

def callback(update, context):
pass

Expand Down Expand Up @@ -513,6 +534,21 @@ def update_chat_data(self, chat_id, data):
def update_user_data(self, user_id, data):
self.test_flag_user_data = True

def update_conversation(self, name, key, new_state):
pass

def get_conversations(self, name):
pass

def get_user_data(self):
pass

def get_bot_data(self):
pass

def get_chat_data(self):
pass

def callback(update, context):
pass

Expand Down
6 changes: 2 additions & 4 deletions tests/test_filters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,10 +730,8 @@ def test_faulty_custom_filter(self, update):
class _CustomFilter(BaseFilter):
pass

custom = _CustomFilter()

with pytest.raises(NotImplementedError):
(custom & Filters.text)(update)
with pytest.raises(TypeError, match='Can\'t instantiate abstract class _CustomFilter'):
_CustomFilter()

def test_custom_unnamed_filter(self, update):
class Unnamed(BaseFilter):
Expand Down
53 changes: 34 additions & 19 deletions tests/test_persistence.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,33 @@ def change_directory(tmp_path):

@pytest.fixture(scope="function")
def base_persistence():
return BasePersistence(store_chat_data=True, store_user_data=True, store_bot_data=True)
class OwnPersistence(BasePersistence):

def get_bot_data(self):
raise NotImplementedError

def get_chat_data(self):
raise NotImplementedError

def get_user_data(self):
raise NotImplementedError

def get_conversations(self, name):
raise NotImplementedError

def update_bot_data(self, data):
raise NotImplementedError

def update_chat_data(self, chat_id, data):
raise NotImplementedError

def update_conversation(self, name, key, new_state):
raise NotImplementedError

def update_user_data(self, user_id, data):
raise NotImplementedError

return OwnPersistence(store_chat_data=True, store_user_data=True, store_bot_data=True)


@pytest.fixture(scope="function")
Expand Down Expand Up @@ -100,22 +126,13 @@ class TestBasePersistence(object):
def test_creation(self, base_persistence):
assert base_persistence.store_chat_data
assert base_persistence.store_user_data
with pytest.raises(NotImplementedError):
base_persistence.get_bot_data()
with pytest.raises(NotImplementedError):
base_persistence.get_chat_data()
with pytest.raises(NotImplementedError):
base_persistence.get_user_data()
with pytest.raises(NotImplementedError):
base_persistence.get_conversations("test")
with pytest.raises(NotImplementedError):
base_persistence.update_bot_data(None)
with pytest.raises(NotImplementedError):
base_persistence.update_chat_data(None, None)
with pytest.raises(NotImplementedError):
base_persistence.update_user_data(None, None)
with pytest.raises(NotImplementedError):
base_persistence.update_conversation(None, None, None)
assert base_persistence.store_bot_data

def test_abstract_methods(self):
with pytest.raises(TypeError, match=('get_bot_data, get_chat_data, get_conversations, '
'get_user_data, update_bot_data, update_chat_data, '
'update_conversation, update_user_data')):
BasePersistence()

def test_implementation(self, updater, base_persistence):
dp = updater.dispatcher
Expand All @@ -127,8 +144,6 @@ def test_conversationhandler_addition(self, dp, base_persistence):
with pytest.raises(ValueError, match="if dispatcher has no persistence"):
dp.add_handler(ConversationHandler([], {}, [], persistent=True, name="My Handler"))
dp.persistence = base_persistence
with pytest.raises(NotImplementedError):
dp.add_handler(ConversationHandler([], {}, [], persistent=True, name="My Handler"))

def test_dispatcher_integration_init(self, bot, base_persistence, chat_data, user_data,
bot_data):
Expand Down
4 changes: 2 additions & 2 deletions tests/test_updater.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@

from telegram import TelegramError, Message, User, Chat, Update, Bot
from telegram.error import Unauthorized, InvalidToken, TimedOut, RetryAfter
from telegram.ext import Updater, Dispatcher, BasePersistence
from telegram.ext import Updater, Dispatcher, DictPersistence

signalskip = pytest.mark.skipif(sys.platform == 'win32',
reason='Can\'t send signals without stopping '
Expand Down Expand Up @@ -467,7 +467,7 @@ def test_mutual_exclude_bot_dispatcher(self):

def test_mutual_exclude_persistence_dispatcher(self):
dispatcher = Dispatcher(None, None)
persistence = BasePersistence()
persistence = DictPersistence()
with pytest.raises(ValueError):
Updater(dispatcher=dispatcher, persistence=persistence)

Expand Down