4
\$\begingroup\$

I recently learned Python using the book by Mark Summerfeld and I am now doing a few katas of mine when I learn a new language. This helps to get the “how the language works” and I wrote a simple lexer for Apache logs.

An example of log records are

64.242.88.10 - - [07/Mar/2004:16:05:49 -0800] "GET /twiki/bin/edit/Main/Double_bounce_sender?topicparent=Main.ConfigurationVariables HTTP/1.1" 401 12846
64.242.88.10 - - [07/Mar/2004:16:06:51 -0800] "GET /twiki/bin/rdiff/TWiki/NewUserTemplate?rev1=1.3&rev2=1.2 HTTP/1.1" 200 4523
64.242.88.10 - - [07/Mar/2004:16:10:02 -0800] "GET /mailman/listinfo/hsdivision HTTP/1.1" 200 6291

and program I wrote reads all values as tokens and output them using | as a separator, as in

64.242.88.10|-|-|07/Mar/2004:16:05:49 -0800|GET /twiki/bin/edit/Main/Double_bounce_sender?topicparent=Main.ConfigurationVariables HTTP/1.1|401|12846
64.242.88.10|-|-|07/Mar/2004:16:06:51 -0800|GET /twiki/bin/rdiff/TWiki/NewUserTemplate?rev1=1.3&rev2=1.2 HTTP/1.1|200|4523
64.242.88.10|-|-|07/Mar/2004:16:10:02 -0800|GET /mailman/listinfo/hsdivision HTTP/1.1|200|6291

The lexer can be used for more general usage though.

Here is my code and I would love to read your comments and suggestions about how to improve it. I have also raised a few specific questions on certain topics.

import collections
import io
import sys

LEXER_SKIP_WHITESPACE=0
LEXER_READ_QUOTED_STRING=1
LEXER_READ_BRACKETED_STRING=2
LEXER_READ_WORD=3

class Location:
    def __init__(self, name=None, pos=0, line=1, col=0):
        self.name = name or "<input>"
        self.line = line
        self.col = col
        self.pos = pos
    def update(self, c):
        self.pos += 1
        if c == "\n":
            self.line += 1
            self.col = 0
        else:
            self.col += 1
    def __repr__(self):
        return str.format("Location({}, {}, {}, {})", repr(self.name), repr(self.pos), repr(self.line), repr(self.col))
    def __str__(self):
        return str.format("{}: {}: line {}, column {}", self.name, self.pos, self.line, self.col)

def readchar(inputchannel, location):
    while True:
        maybechar = inputchannel.read(1)
        if maybechar == '':
            return None
        else:
            location.update(maybechar)
            yield maybechar

def readtoken(inputchannel, location):
    state = LEXER_SKIP_WHITESPACE
    token = ''
    for nextchar in readchar(inputchannel, location):
        if state is LEXER_SKIP_WHITESPACE:
            if nextchar == "\n":
                yield "\n"
                continue
            elif nextchar.isspace():
                continue
            elif nextchar == '"':
                state = LEXER_READ_QUOTED_STRING
                continue
            elif nextchar == '[':
                state = LEXER_READ_BRACKETED_STRING
                continue
            else:
                state = LEXER_READ_WORD
                token += nextchar
                continue
        elif state is LEXER_READ_QUOTED_STRING:
            if nextchar == '"':
                yield token
                token = ''
                state = LEXER_SKIP_WHITESPACE
                continue
            else:
                token += nextchar
                continue
        elif state is LEXER_READ_BRACKETED_STRING:
            if nextchar == ']':
                yield token
                token = ''
                state = LEXER_SKIP_WHITESPACE
                continue
            else:
                token += nextchar
                continue
        elif state is LEXER_READ_WORD:
            if nextchar == "\n":
                yield token
                token = ''
                state = LEXER_SKIP_WHITESPACE
                yield "\n"
                continue
            elif nextchar.isspace():
                yield token
                token = ''
                state = LEXER_SKIP_WHITESPACE
                continue
            else:
                token += nextchar
                continue
        else:
            raise Error("Impossible lexer state.")
    if state is LEXER_SKIP_WHITESPACE:
        return None
    elif state is LEXER_READ_QUOTED_STRING:
        raise Error("End of character stream in quoted string.")
    elif state is LEXER_READ_BRACKETED_STRING:
        raise Error("End of character stream in quoted string.")
    elif state is LEXER_READ_WORD:
        yield token
        return None
    else:
        raise Error("Impossible lexer state.")


class Lexer:
    def __init__(self, inputchannel, _location=None):
        self.location = _location or Location("<input>", 0, 1, 0)
        self.inputchannel = inputchannel
        self.buf = ''
        self.state = LEXER_SKIP_WHITESPACE

    def __iter__(self):
        return readtoken(self.inputchannel, self.location)

if __name__ == "__main__":
    sep = ''
    for token in Lexer(sys.stdin):
        if token == '\n':
            sys.stdout.write(token)
            sep = ''
        else:
            sys.stdout.write(sep)
            sys.stdout.write(token)
            sep = '|'

Question 1 – How to write lexers in Python?

I am well aware that there are tools similar to lex and yacc for Python that could have been used here, but that would defeat the purpose of learning how to write such a program in Python. I found it surprisingly hard.

My first misfortune is that Python does not do tail-elimination, so it is essentially forbidden to write a lexer as a set of mutually recursive functions. Mutually recursive functions are one of my favourite tools to do so, because it clearly singles out a specific state of the lexer (the recursive function it is in) and the transitions from this state to the other states, and makes it easy to test-case individually each of the transitions.

Since maybe not everybody is familiar with lexers based on mutually recursive functions here is the equivalent of the readtoken generator in OCaml. The beginning of read_token reads like "If you read a double quote, discard it and do read_quotedstring" and that function itself is defined later and does what on expects: it aggregates characters in a buffer until the next double quote and bless the result as a token.

let rec read_token f ((ax,buffer) as state) s =
  match Stream.peek s with
  | Some('"') -> Stream.junk s; read_quotedstring f state s
  | Some('[') -> Stream.junk s; read_timestamp f state s
  | Some(' ')
  | Some('\t')-> Stream.junk s; read_token f state s
  | Some(c) -> read_simpleword f state s
  | None -> ax
and read_simpleword f ((ax,buffer) as state) s =
  match Stream.peek s with
  | Some('"')
  | Some('[')
  | Some(' ')
  | Some('\t') ->
    let current_token = Buffer.contents buffer in
    Buffer.clear buffer;
    read_token f (f ax current_token, buffer) s
  | Some(c) ->
    Buffer.add_char buffer c;
    Stream.junk s;
    read_simpleword f state s
  | None ->
    ax
and read_quotedstring f ((ax,buffer) as state) s =
  match Stream.peek(s) with
  | Some('"') ->
    Stream.junk s;
    let current_token = Buffer.contents buffer in
    Buffer.clear buffer;
    read_token f (f ax current_token, buffer) s
  | Some(c) ->
    Stream.junk s;
    Buffer.add_char buffer c;
    read_quotedstring f state s
  | None ->
    failwith "End of stream within a quoted string"
and read_timestamp f ((ax,buffer) as state) s =
  match Stream.peek(s) with
  | Some(']') ->
    Stream.junk s;
    let current_token = Buffer.contents buffer in
    Buffer.clear buffer;
    read_token f (f ax (timestamp_to_iso current_token), buffer) s
  | Some(c) ->
    Stream.junk s;
    Buffer.add_char buffer c;
    read_timestamp f state s
  | None ->
    failwith "End of stream within a bracketed string"

My Python version defines a few states as symbolic constants as I would have done in C and then build a huge while loop keeping track of the transitions. I am not please with that implementation, because it does not give me any tool to manage the complexity of the lexer. Working with functions is very unpleasant because of the details of the scope of variables in Python. So what would be the idiomatic way to break down the lexer in small testable pieces, which would be important if I would decide to write a more complicate parser? Could the idea to represent lexer-states by objects be interesting?

Question 2. Is it right to treat stdin as a character stream?

Obviously I somehow did that but Python has no real character type and makes them look like length 1 strings. It feels to me that the approach of reading my input in chunks into “circular expandable buffers” and making copies of substrings of these chunks to generate my tokens would fit much more nicely in the language. Am I right?

Question 3. What is the all-purpose exception in Python?

This seems like a rather basic question but I am really failing to find a suitable answer in the documentation The Exception seems a poor choice, since it very general and make error identification and error handling rather complicated.

Question 4. Do generators return none?

Is it good style to return None in generators? Would pass also do?

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

In addition to the other answers, I wanted to add my own thoughts on Q1 and Q4.

Question 1

Break up readtoken into multiple functions.

Right now, you are using a state machine in a single recursive function to try and read different types of tokens. As mentioned in another answer, recursion is not very "pythonic". Additionally, it is hard to follow the logic in this function since the state changes in each recursion.

This logic would be easier to understand if you break the different states up into separate functions, as it appears you have done in your OCaml example. You can even use similar high-level function names. We can then change our definition of readtoken to to route to other helper functions based on the first character we read from the stream.

def readtoken(inputchannel, location):
    for nextchar in readchar(inputchannel, location):
        if nextchar == '\n':
            yield '\n'
        elif nextchar.isspace():
            continue
        elif nextchar == '"':
            yield read_quoted_string(inputchannel, location)
        elif nextchar == '[':
            yield read_bracketed_string(inputchannel, location)
        else:
            yield read_token(nextchar, inputchannel, location)

Note how the function immediately becomes easier to read. We are delegating our responsibilities out to other named functions, just like in your OCaml example. The purpose of readtoken is now simply that of a router, where we are delegating work out to other functions based on the first character we read from the stream.

Each of these helper functions now has a single responsibility - read their given token type from the stream, and return the result. Note that these functions should actually return a result, and not yield it - these functions are not generators, the only generator here is the readtoken function.

Helper function implementations:

def read_quoted_string(inputchannel, location):
    token = ''
    for nextchar in readchar(inputchannel, location):
        if next_char == '"':
            return token
        else:
            token += nextchar
    raise Exception("End of character stream in quoted string.")


def read_bracketed_string(inputchannel, location):
    token = ''
    for nextchar in readchar(inputchannel, location):
        if nextchar == ']':
            return token
        else:
            token += nextchar
    raise Exception("End of character stream in bracketed string.")


def read_token(token, inputchannel, location):
    for nextchar in readchar(inputchannel, location):
        if nextchar.isspace():
            return token
        else:
            token += nextchar
    return token # if we reach the end of the stream

Take note of a few things with this implementation:

  • Each function becomes much more readable. We no longer have to trace through a single recursive function and keep track of the state it is in.
  • We can now see that read_quoted_string and read_bracketed_string have a lot of common logic. Perhaps we should combine these into a single function read_string with a string_delimiter as a parameter.
  • read_token takes an initial token as its first parameter - this is the character that we read from the top-level readtokens function. If we don't like this implementation, we could try to solve it by using peek at the top-level instead.

Don't make readtoken a generator function.

In your example, the Lexer class has implemented __iter__ which treats it as an iterable. This means that your readtoken does not need to be a generator function (i.e you can replace all yield statements with return), because the Lexer class is already a generator that wraps around it.

Question 4

When a generator function exits, it automatically raises a StopIteration. for-loops and while-loops will automatically handle this StopIteration by ending their loops and continuing on.

Therefore, the "normal" way to end a generator is to simply end the function. return None does accomplish this, but a more traditional method would be to simply reach the end of the function and return naturally. You can accomplish this by breaking out of the while-loop in your readchar function.

\$\endgroup\$
4
+50
\$\begingroup\$

Note: I'm assuming that this code works and you seek only 'code quality' improvements. Also, you seem like experienced programmer so I will try not to explain trivial things but please, do ask if you think I didn't state my opinion clearly enough.

Code notes:

General

No docs on any method or class. While methods are descriptive one might have trouble with for example, what is being updated in Location.update, it might be a name update as well.

In my opinion both readchar and readtoken should be wrapped into some class - a Reader maybe? Advantages of that are (at least) two: you do not clutter global namespace with functions and you could store both inputchannel, state and location as a class variables.

Imports

import collections
import io
import sys

I wouldn't import whole packages. Try to using from X import Y pattern, it is both more descriptive (reader can see why are you using those packages) and also you don't have to repeat yourself (as you do with sys.stdout). However, this is just a matter of preference.

Global (file) variables

I tend to move them into separate file and import them. It is mainly because of python's zen:

Namespaces are one honking great idea -- let's do more of those!

Moving your constant's away from the usage helps in the future if you would need to make them configurable - it separates 'implementation' from the usage.

Also, (if you happen to use python 3.4+) it might be beneficial to use enums instead. In addition to creating new 'namespace' it also provides a very important feature- your globals will be constant.

Location's init

I must say that I love a fact that you used name or "<input>", people tend to check for None with if too much.

Location's update

The c variable is not very descriptive, what you probably meant was 'character'/'char' ?

Location's repr and str

I personally don't like when people use str.format method it has no advantage over "Location({}, {}, {}, {})".format but is longer. However, if you target your classes for python 3.6+ why not using something even more readable: string interpolation.

Location's readchar

Instead of returning None I'd raise StopIterationError. It is because it is the usual thing to do when you want to stop a generator.

As @Maarten Fabré pointed out, this is no longer valid. Instead, just use return (see Explanation of generators, iterators, and StopIteration )

readtoken

  • In my opinion, this method is too long, I'd split it based on state, after each state is ... you'd call different function.
  • Depending on your needs it might be useful to use linesp instead of "\n".
  • Do not raise generic Exception, more on that later.
  • As previously, do not return none - raise an exception instead.

Lexer's init

  • In this case a documentation would be lovely, particularly to understand what inputchannel is and what API must it comply to.

  • In Location("<input>", 0, 1, 0) it would be good to name the parameters that you pass so reader wouldn't need to search for signature: Location(name="<input>", pos=0, line=1, col=0).

  • What does for token in Lexer(sys.stdin): mean? Will the reader understand what does it mean to 'iterate over lexer' ? My preference would be to rearrange your class to something like for token in lexer.Tokenize(sys.stdin).

Questions

Q1

You are right, recursion is generally discouraged in python. In general, what I saw in similar use cases people do tend to scan char by char as you did. I don't have much experience in writing lexers so I am not sure if my advice will be at all helpful but let's try:

  • Small functions are your friend - the more logic you abstract the more complexity you can put in. For example, instead of doing x == '[' try isLeftBracket(x), in the future you might introduce isBrachet which will call isLeftBracet or isRightBracket. This is just a silly example but I hope you get the point.
  • Instead of recursion how about using a stack of states? It is a general practice to change recursive code to iterative one.
  • You could abstract your class to a state machine (basically wrapping everything as I mentioned at the beginning), this will help in managing the state and include many events as functions.
  • First consume and then validate multiple characters given a state. For example, if you know that if current state is A and you encounter character [, next few characters are going to be some specific data

you could do this:

if state == A and current_character == '[':
        consume_dataX()
def consume_dataX():
    a = ''
    while condition(current_character):
        a += readchar()
        current_character = readchar()
    validate(a) 

Real life example would be parsing a function (foo(int x, int y)):

if state == ParsingFunction and current_character == "(":
   argument_vector = consume_until(')')

Q2

It is completely fine to think of characters as a length 1 strings. You can think of it like this: for a given string a operator [] is a slicing operator which means that a[1] slices the string and is short for a[1:2]. Also, keep in mind that in reality char type is just a byte (or something similar) therefore, there is no need for a special treatment.

Q3

I cannot stress this enough, in my opinion: DO NOT RAISE GENERIC EXCEPTIONS unless you have completely no idea what happened.

Person using your class will have to do the following:

try:
    # some code
except Exception as ex:
    # handle

which is inconvenient for many reason. If you want to know more see this SO question.

Q4

Already answered above.

\$\endgroup\$
2
  • 2
    \$\begingroup\$ about the StopIteration: pep-479: This PEP proposes a change to generators: when StopIteration is raised inside a generator, it is replaced it with RuntimeError. (More precisely, this happens when the exception is about to bubble out of the generator's stack frame.) \$\endgroup\$ Sep 18, 2018 at 8:36
  • \$\begingroup\$ @MaartenFabré thank you for that, I guess we learn something new every day :) \$\endgroup\$
    – MaLiN2223
    Sep 18, 2018 at 10:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.