rougeth / bottery

[DEVELOPMENT-HALTED] :battery: A bot framework with batteries included
MIT License
344 stars 51 forks source link

Allow more complex patterns (without context) #81

Closed rougeth closed 6 years ago

rougeth commented 7 years ago

At Python Brasil, we talked a lot about how Pattern obj is so limited and how we could improve it. The main point of the discussion was how we could implement other "types" of patterns:

patterns = [
    # 1)
    Pattern('ping', ping),
    RegexPattern(r'^ping \d*$', regex_ping), # "ping 123" would be accepted
    StartswithPattern('/ping', regex_ping), # "/ping bottery" would be accepted

    # 2) or
    Pattern('ping', ping),
    Pattern(r'^ping$', regex_ping, type='regex'),
    Pattern('/ping', regex_ping, type='startswidth'),

    # 3) or
    Pattern('ping', ping),
    Pattern(r'^ping$', regex_ping, validator=regex_validator),
    Pattern('/ping', regex_ping, validator=startswith_validator),
    # where regex_validator and startswith_validator are the checkers
]

I believe the second "kind of" pattern is the easiest for who will use Bottery. But I think we should discuss it better.

@nicoddemus @muriloviana @mawkee @ElSaico @leportella

ElSaico commented 7 years ago

I'm partial towards 2) as well, myself - though the available mappings for type would need to be a configurable setting for extensiblity purposes.

As an aside, Django 2.0 is going to follow option 1), with the path and re_path functions:

https://docs.djangoproject.com/en/dev/releases/2.0/#simplified-url-routing-syntax

leportella commented 7 years ago

Although I think that the second (2) option seems as an easier way to remember things, I personally prefer the first one.

I think the first option would be easier to remember, easier to document and generates lines with less characters, which makes it more readable. Also, you can explore and individualize each of the patterns very well, which seems easier for me.

nicoddemus commented 7 years ago

I think 2) is more suitable, as it will be more flexible. For example, when we include NLP support the pattern constructor will probably be very different:

NaturalPattern([PURPOSE.BUY, PURPOSE.PRODUCTS], purchase_products)

IOW, not all patterns will share the same "constructor".

It will also allow external packages to easily add patterns themselves without the need for any special support from bottery other than a well defined interface that patterns should comply to.

leportella commented 7 years ago

@nicoddemus you said you prefer the second one, but your example seems to be equal to the first option. Which one do you prefer? :D

nicoddemus commented 7 years ago

Oops sorry I meant the first one, with separate classes for each pattern. 😋

muriloviana commented 7 years ago

The option 2) looks pretty easy to understand and manage. However, I think the option 1) is more explicit and readable to Bottery users.

muriloviana commented 7 years ago

I really like the idea of keep only one Pattern, but I guess it looks too magical.

mawkee commented 7 years ago

I think I prefer the first option. This would give us more flexibility for extra attributes depending on the pattern class; for instance, it'd make sense for RegexPattern to be able to use named groups, but that would make zero sense for Pattern. It'd be easier for IDEs to show some hints based on the pattern class

Even though 2 could potentially be easier to implement, on the long run it could increase complexity.

rougeth commented 7 years ago

Wooh, internally, Django does something really similar with the third option: https://github.com/django/django/blob/master/django/urls/conf.py#L76-L77

mawkee commented 7 years ago

I think it's actually similar to the first option. They implemented RoutePattern and RegexPattern, then created a partial to each one of them (path / re_path). In practice, for the common user, he'll use either one of those

mawkee commented 7 years ago

I can work on this as soon as you decide which approach you want done ;-)

rougeth commented 7 years ago

@mawkee well, let's try the first option :)

IvanBrasilico commented 7 years ago

A suggestion of a FlexiblePattern. User can pass any function to pre-process the message beffore comparing to pattern:

class FuncPattern(Pattern): '''Receives a function to preprocess the incoming message text before comparing it to the pattern. Allows use of regular expressions, selecting partial words for routing, etc''' def init(self, pattern, view, pre_process): self.pre_process = pre_process Pattern.init(self, pattern, view)

def check(self, message):
    text, _ = self.pre_process(message.text)
    if text == self.pattern:
        return self.view
    return False

A function, for example, similar to starts_with:

def two_tokens(text): '''Receives a text string, splits on first space, return first word of list/original sentence and the rest of the sentence ''' lista = text.split(' ') return lista[0], " ".join(lista[1:])

def sumfunction(message): , number_list = two_tokens(message.text) mysum = 0 for item in number_list: mysum += int(item) return str(mysum)

Usage: FuncPattern('\sum', sum_function, two_tokens),

User type on Bot:

/sum 1 2 3

Receives:

6

IvanBrasilico commented 7 years ago

Code on https://github.com/IvanBrasilico/LacreBot

leportella commented 6 years ago

I think that after the refactor this issue should be closed, since patterns changed drastically