Page MenuHomePhabricator

evenstreams filter doesn't register multiple arguments at one
Closed, ResolvedPublic

Description

Sample:

>>> from pywikibot.comms.eventstreams import EventStreams
>>> stream = EventStreams(stream='recentchange')
>>> stream.register_filter(foo=True, bar='baz')
>>> f, g = stream.filter['all']
>>> c = {'foo': True}
>>> f(c), g(c)
(False, False)
>>> c = {'bar': 'baz'}
>>> f(c), g(c)
(True, True)
>>> del streams

Expected result:

>>> f(c), g(c)
(True, False)
>>> c = {'bar': 'baz'}
>>> f(c), g(c)
(False, True)
>>>

whereas registering in single arguments works:

>>> stream = EventStreams(stream='recentchange')
>>> stream.register_filter(foo=True)
>>> stream.register_filter(bar='baz')
>>> f, g = stream.filter['all']
>>> c = {'foo': True}
>>> f(c), g(c)
(True, False)
>>> c = {'bar': 'baz'}
>>> f(c), g(c)
(False, True)
>>>

Event Timeline

Xqt triaged this task as High priority.Mar 3 2018, 5:47 PM

I know the solution (appending the function by another method like

def do_it:
  self.filter[ftype].append(lambda e: key in e and e[key] == value)

for key, value in kwargs:
   do_it(ftype, key, value)

) but I have no glue why the current implementation doesn't work.

I have no glue why the current implementation doesn't work.

Variable scopes.

diff --git a/pywikibot/comms/eventstreams.py b/pywikibot/comms/eventstreams.py
index a74baa1..e5ccd9c 100644
--- a/pywikibot/comms/eventstreams.py
+++ b/pywikibot/comms/eventstreams.py
@@ -219,15 +219,15 @@ class EventStreams(object):
         for key, value in kwargs.items():
             # append function for singletons
             if isinstance(value, (bool, type(None))):
-                self.filter[ftype].append(lambda e: key in e and
+                self.filter[ftype].append(lambda e: print(e, key, value) or key in e and
                                           e[key] is value)
             # append function for a single value
             elif isinstance(value, (StringTypes, int)):
-                self.filter[ftype].append(lambda e: key in e and
+                self.filter[ftype].append(lambda e: print(e, key, value) or key in e and
                                           e[key] == value)
             # append function for an iterable as value
             else:
-                self.filter[ftype].append(lambda e: key in e and
+                self.filter[ftype].append(lambda e: print(e, key, value) or key in e and
                                           e[key] in value)
 
     def streamfilter(self, data):
$ PYWIKIBOT2_NO_USER_CONFIG=1 python3 pwb.py shell
Skipping loading of user-config.py.
family and mylang are not set.
Defaulting to family='test' and mylang='test'.
Welcome to the Pywikibot interactive shell!
>>> from pywikibot.comms.eventstreams import EventStreams
>>> stream = EventStreams(stream='recentchange')
>>> stream.register_filter(foo=True, bar='baz')
>>> f, g = stream.filter['all']
>>> c = {'foo': True}
>>> f(c), g(c)
{'foo': True} bar baz
{'foo': True} bar baz
(False, False)
>>> c = {'bar': 'baz'}
>>> f(c), g(c)
{'bar': 'baz'} bar baz
{'bar': 'baz'} bar baz
(True, True)
>>> del stream

key and value belong to the closure of the outer function, not the loop iteration or the inner lambda.

One way to solve this without using more lambdas:

diff --git a/pywikibot/comms/eventstreams.py b/pywikibot/comms/eventstreams.py
index a74baa1..b48ecc8 100644
--- a/pywikibot/comms/eventstreams.py
+++ b/pywikibot/comms/eventstreams.py
@@ -219,16 +219,16 @@ class EventStreams(object):
         for key, value in kwargs.items():
             # append function for singletons
             if isinstance(value, (bool, type(None))):
-                self.filter[ftype].append(lambda e: key in e and
-                                          e[key] is value)
+                self.filter[ftype].append(lambda e, _key=key, _value=value:
+                                          _key in e and e[_key] is _value)
             # append function for a single value
             elif isinstance(value, (StringTypes, int)):
-                self.filter[ftype].append(lambda e: key in e and
-                                          e[key] == value)
+                self.filter[ftype].append(lambda e, _key=key, _value=value:
+                                          _key in e and e[_key] == _value)
             # append function for an iterable as value
             else:
-                self.filter[ftype].append(lambda e: key in e and
-                                          e[key] in value)
+                self.filter[ftype].append(lambda e, _key=key, _value=value:
+                                          _key in e and e[_key] in _value)
 
     def streamfilter(self, data):
         """Filter function for eventstreams.
$ PYWIKIBOT2_NO_USER_CONFIG=1 python3 pwb.py shell
Skipping loading of user-config.py.
family and mylang are not set.
Defaulting to family='test' and mylang='test'.
Welcome to the Pywikibot interactive shell!
>>> from pywikibot.comms.eventstreams import EventStreams
>>> stream = EventStreams(stream='recentchange')
>>> stream.register_filter(foo=True, bar='baz')
>>> f, g = stream.filter['all']
>>> c = {'foo': True}
>>> f(c), g(c)
(True, False)
>>> c = {'bar': 'baz'}
>>> f(c), g(c)
(False, True)
>>> del stream

I think it is because in

for key, value in kwargs.items():
            # append function for singletons
            if isinstance(value, (bool, type(None))):
                self.filter[ftype].append(lambda e: key in e and e[key] is value)
...

the last key, value pair is the only one that will be used in all lambdas.
Try just printing the key:

self.filter[ftype].append(lambda e: key)
In [59]: %paste
from pywikibot.comms.eventstreams import EventStreams
stream = EventStreams(stream='recentchange')
stream.register_filter(foo=True, bar='baz')
f, g = stream.filter['all']
c = {'foo': True}
f(c), g(c)

## -- End pasted text --
Out[59]: ('bar', 'bar')

I would replace the lambda with a real function.
Something like:

import functools
def _do(e, key=None, value=None):
    return key in e and e[key] is value
    
# register pairs of keys and items as a filter function
for key, value in kwargs.items():
    # append function for singletons
    if isinstance(value, (bool, type(None))):
        self.filter[ftype].append(functools.partial(_do, key=key, value=value))

In [57]: f
Out[57]: functools.partial(<function EventStreams.register_filter.<locals>._do at 0xaaa17974>, key='foo', value=True)

In [58]: g
Out[58]: functools.partial(<function EventStreams.register_filter.<locals>._do at 0xaaa17974>, key='bar', value='baz')

OK, I didn't see zhuyifei1999 answered as well while I was writing :-)
I think partial is better than lambda as it gives a clearer inspection than:

In [61]: f
Out[61]: <function pywikibot.comms.eventstreams.EventStreams.register_filter.<locals>.<lambda>>

In [62]: g
Out[62]: <function pywikibot.comms.eventstreams.EventStreams.register_filter.<locals>.<lambda>>

Change 416239 had a related patch set uploaded (by Xqt; owner: Xqt):
[pywikibot/core@master] [bugfix] Enable EventStreams filter for multiple arguments

https://gerrit.wikimedia.org/r/416239

@Mpaa: Thanks for your proposal but the unittest fails then:

FAIL: test_filter_settings (__main__.TestEventStreamsSettingTests)
Test EventStreams filter settings.
----------------------------------------------------------------------
Traceback (most recent call last):
  File ".\tests\eventstreams_tests.py", line 142, in test_filter_settings
    self.assertIsInstance(self.es.filter['all'][0], FunctionType)
AssertionError: functools.partial(<function EventStreams.register_filter.<locals>._do_equal at 0x05287390>, key='foo', value='bar') is not an instance of <class 'function'>

the code snipped was

...
        def _do_is(e, key=None, value=None):
            return key in e and e[key] is value
        def _do_equal(e, key=None, value=None):
            return key in e and e[key] == value
        def _do_in(e, key=None, value=None):
            return key in e and e[key] in value
...
        for key, value in kwargs.items():
            # append function for singletons
            if isinstance(value, (bool, type(None))):
                self.filter[ftype].append(
                    partial(_do_is, key=key, value=value))                
            # append function for a single value
            elif isinstance(value, (StringTypes, int)):
                self.filter[ftype].append(
                    partial(_do_equal, key=key, value=value))                
            # append function for an iterable as value
            else:
                self.filter[ftype].append(
                    partial(_do_in, key=key, value=value))

changed test to callable(...)

Change 416239 merged by jenkins-bot:
[pywikibot/core@master] [bugfix] Enable EventStreams filter for multiple arguments

https://gerrit.wikimedia.org/r/416239