Dropping non context by Poolitzer · Pull Request #2617 · python-telegram-bot/python-telegram-bot

Poolitzer · 2021-08-04T10:21:49Z

This PR drops all non context based code. It also removes the regex handler (because it had non context code in it and just removing that one from the handler which was going to be removed anyway felt unnecessary).

Also deleted the regex handler

Poolitzer · 2021-08-04T10:22:40Z

The tests will fail. I have no clue about mypy (help pls) and my internet connection is too unstable rn to run pytest, so I am going to use the CI results to fix the remaining tests.

telegram/ext/handler.py:144: error: Argument 1 to "collect_additional_context" of "Handler" has incompatible type "Optional[CCT]"; expected "CCT"  [arg-type]
telegram/ext/handler.py:147: error: Argument 2 has incompatible type "Optional[CCT]"; expected "CCT"  [arg-type]
telegram/ext/conversationhandler.py:645: error: Argument 4 to "_schedule_job" of "ConversationHandler" has incompatible type "Optional[CallbackContext[Any, Any, Any]]"; expected "CallbackContext[Any, Any, Any]"  [arg-type]

Now that the test suite actually ran through.

Poolitzer · 2021-08-04T11:42:49Z

Apart from the mypy errors mentioned above which I do not understand, I also dont understand this pytest error, all others pass \o/

__________________ TestDispatcher.test_one_context_per_update __________________

self = <tests.test_dispatcher.TestDispatcher object at 0x7f90e4361898>
dp = <telegram.ext.dispatcher.Dispatcher object at 0x7f90e50fcd90>

    def test_one_context_per_update(self, dp):
        def one(update, context):
            if update.message.text == 'test':
                context.my_flag = True
    
        def two(update, context):
            if update.message.text == 'test':
                if not hasattr(context, 'my_flag'):
                    pytest.fail()
            else:
                if hasattr(context, 'my_flag'):
                    pytest.fail()
    
        dp.add_handler(MessageHandler(Filters.regex('test'), one), group=1)
        dp.add_handler(MessageHandler(None, two), group=2)
        u = Update(1, Message(1, None, None, None, text='test'))
>       dp.process_update(u)

tests/test_dispatcher.py:142: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
telegram/ext/dispatcher.py:538: in process_update
    handler.handle_update(update, self, check, context)
telegram/ext/handler.py:147: in handle_update
    return self.callback(update, context)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

update = <telegram.update.Update object at 0x7f90e4248e88>
context = <telegram.ext.callbackcontext.CallbackContext object at 0x7f90e40d89a8>

    def two(update, context):
        if update.message.text == 'test':
            if not hasattr(context, 'my_flag'):
>               pytest.fail()
E               Failed

tests/test_dispatcher.py:134: Failed

Bibo-Joshi

Overall looks good! A few notes:

Searching for use_context in the branch still gives a number of results - please make sure to remove those
I only skimmed the tests, but I'm sure they are fine :)
Coverage decrease: Codecov marked nothing as untested that was tested before, IISC. I guess the decrease is just due to the fact that we're removing a lot of testing lines here so that the percentage of untested lines increases …

telegram/ext/__init__.py

Bibo-Joshi · 2021-08-12T14:46:52Z

👍
Just saw that most docstrings for callback arguments say something like Callback signature for context based API. I guess we can simplify that now …

Poolitzer · 2021-08-12T15:15:26Z

@Bibo-Joshi True, good point

harshil21

noticed that regexhandler needs to be removed from telegram.ext.rst too. Skimmed through the rest, seems fine.

Poolitzer: Grrr, I swear I searched for regexhandler in the project

Poolitzer · 2021-08-19T14:25:58Z

I tried fixing the merge conflicts but I might have broken everything :(

github-actions

Hey there. Relax, I am just a little warning for the maintainers to release directly after merging your PR, otherwise we have broken examples and people might get confused :)

# Conflicts: # tests/test_dispatcher.py # tests/test_persistence.py

Bibo-Joshi · 2021-08-19T17:44:33Z

sorry to be bummer, but I think you were a little too thorough with your last doc change. IMO we should still state how the signature of the callback looks like, either by writing def callback(…) or just writing something like "the callback must be callable with two positional arguments of type Update and CallbackContext, respectively". What I meant was that we can skip the "for context based API" part, is there is no different API anymore now :)

Poolitzer · 2021-08-19T18:30:49Z

Hmm. Yeah I guess you are right, no way to know this without looking at the wiki.

# Conflicts: # telegram/ext/handler.py # telegram/ext/jobqueue.py # tests/test_callbackcontext.py # tests/test_callbackqueryhandler.py # tests/test_chatmemberhandler.py # tests/test_inlinequeryhandler.py # tests/test_messagehandler.py # tests/test_pollanswerhandler.py # tests/test_pollhandler.py # tests/test_precheckoutqueryhandler.py # tests/test_regexhandler.py # tests/test_shippingqueryhandler.py # tests/test_stringcommandhandler.py # tests/test_stringregexhandler.py # tests/test_typehandler.py

Bibo-Joshi and others added 3 commits July 24, 2021 17:18

Temporarily enable tests for the v14 branch

3f2645f

Improved: MyPy doesn't need that

aefbc4d

Improvement: Removed non context

a8fcd54

Also deleted the regex handler

Poolitzer added 2 commits August 4, 2021 13:15

Fix: Fix updater test + file

bc80950

Fix: Hopefully fix remaining tests

c972752

Now that the test suite actually ran through.

Bibo-Joshi mentioned this pull request Aug 10, 2021

Deprecation: v14 #2346

Closed

22 tasks

Bibo-Joshi added 2 commits August 10, 2021 20:30

Update signature of handle_update

e63981f

Fix "one context per update"

c620dd1

Bibo-Joshi self-requested a review August 10, 2021 18:48

Bibo-Joshi force-pushed the v14 branch from 3f2645f to c3774c6 Compare August 11, 2021 06:45

Bibo-Joshi requested changes Aug 11, 2021

View reviewed changes

telegram/ext/__init__.py Show resolved Hide resolved

Bibo-Joshi and others added 5 commits August 11, 2021 17:47

Small typing improvement

9c81e24

Merge remote-tracking branch 'origin/v14' into dropping_non_context

6675cf8

Fix for previous commit

9517ee5

Fix: Remove RegexHandler from docs

eabc6f0

Fix: Remove last mentions of use_context

9b3f7d7

harshil21 requested changes Aug 12, 2021

View reviewed changes

harshil21 added this to the v14 milestone Aug 13, 2021

harshil21 added the 🛠 breaking change type: breaking label Aug 13, 2021

Poolitzer added 2 commits August 13, 2021 12:02

Fix: Last mention of the regexhandler.

c76fa40

Fix: Remove old docstrings

d2010aa

github-actions bot reviewed Aug 19, 2021

View reviewed changes

Merge branch 'v14' into dropping_non_context

6959ae3

# Conflicts: # tests/test_dispatcher.py # tests/test_persistence.py

Bibo-Joshi force-pushed the dropping_non_context branch from b319bae to 6959ae3 Compare August 19, 2021 17:32

Poolitzer and others added 2 commits August 27, 2021 15:16

Fix: Adding callback signature to docstring

3d8fc09

Bibo-Joshi merged commit a32851a into v14 Aug 29, 2021

Bibo-Joshi deleted the dropping_non_context branch August 29, 2021 16:15

github-actions bot locked and limited conversation to collaborators Aug 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dropping non context#2617

Dropping non context#2617
Bibo-Joshi merged 17 commits intov14from
dropping_non_context

Poolitzer commented Aug 4, 2021

Uh oh!

Poolitzer commented Aug 4, 2021 •

edited

Loading

Uh oh!

Poolitzer commented Aug 4, 2021

Uh oh!

Bibo-Joshi left a comment

Uh oh!

Uh oh!

Bibo-Joshi commented Aug 12, 2021

Uh oh!

Poolitzer commented Aug 12, 2021

Uh oh!

harshil21 left a comment •

edited by Poolitzer

Loading

Uh oh!

Poolitzer commented Aug 19, 2021

Uh oh!

github-actions bot left a comment

Uh oh!

Bibo-Joshi commented Aug 19, 2021

Uh oh!

Poolitzer commented Aug 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

Poolitzer commented Aug 4, 2021

Uh oh!

Poolitzer commented Aug 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Poolitzer commented Aug 4, 2021

Uh oh!

Bibo-Joshi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Bibo-Joshi commented Aug 12, 2021

Uh oh!

Poolitzer commented Aug 12, 2021

Uh oh!

harshil21 left a comment • edited by Poolitzer Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Poolitzer commented Aug 19, 2021

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Bibo-Joshi commented Aug 19, 2021

Uh oh!

Poolitzer commented Aug 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Poolitzer commented Aug 4, 2021 •

edited

Loading

harshil21 left a comment •

edited by Poolitzer

Loading