8
\$\begingroup\$

I've a function to split text on punctuation and leave the rest of the string (including whitespace in other items of the list):

import unicodedata

def split_on_punc(text):
    chars = list(text)
    i = 0
    start_new_word = True
    output = []
    while i < len(chars):
        char = chars[i]
        if is_punctuation(char):
            output.append([char])
            start_new_word = True
        else:
            if start_new_word:
                output.append([])
            start_new_word = False
            output[-1].append(char)
        i += 1
    ##print(output)
    return ["".join(x) for x in output]

def is_punctuation(char):
    return True if unicodedata.category(char).startswith("P") else False

E.g.

split_on_punctuation("This, this a sentence. with lotsa' puncts!?@ hahaha looo world")

[out]:

['This',
 ',',
 ' this a sentence',
 '.',
 ' with lotsa',
 "'",
 ' puncts',
 '!',
 '?',
 '@',
 ' hahaha looo world']

It looks like a very complicated way to check through each character and then keep track of whether it's a start of a new word or not.

I'm looking for improvements in speed and simplicity.

\$\endgroup\$
14
\$\begingroup\$

Billy Mays here for the regex library!

It's API-compatible with the standard Python re module. It's powered by open source, and it's safe for colored fabrics and carpets!

The regex library offers things like \p{Punctuation}, which is actually a shorthand form of p{Punctuation=Yes} which is really a shortening of p{General_category=Punctuation}.

If you can make a Unicode query, regex supports it. Identifiers, Categories, Blocks, Diacritical Marks - it even does Coptic!

It cleans! It brightens! It eliminates odors! All at the same time!

test_data = "This, this a sentence. with lotsa' puncts!?@ hahaha looo world"
print(f"Test data: '{test_data}'")

import regex

PUNCT_RE = regex.compile(r'(\p{Punctuation})')    

print(PUNCT_RE.split(test_data))

The output looks like:

Test data: 'This, this a sentence. with lotsa' puncts!?@ hahaha looo world'
['This', ',', ' this a sentence', '.', ' with lotsa', "'", ' puncts', '!', '', '?', '', '@', ' hahaha looo world']

regex converts your code from a whopping 21 lines to a 1-line method call-- a 2000% reduction! But you gotta call now!

Here's how to order:

pip install regex
\$\endgroup\$
  • \$\begingroup\$ Is unicodedata.category(char).startswith("P") the same as regex's \p{Punctuation}? \$\endgroup\$ – alvas Oct 4 '19 at 3:47
  • 1
    \$\begingroup\$ Is the capturing group required in (\p{Punctuation})? If it isn't required, isn't it better to just do \p{Punctuation} instead? (aka: no capturing group). \$\endgroup\$ – Ismael Miguel Oct 4 '19 at 10:25
  • 3
    \$\begingroup\$ "I used to have a problem, so I used a regular-expression, now I have two problems." \$\endgroup\$ – Dai Oct 4 '19 at 10:52
  • 2
    \$\begingroup\$ The capturing group is required, @IsmaelMiguel - otherwise the punctuation itself would be discarded in the split. \$\endgroup\$ – rolfl Oct 4 '19 at 13:44
  • 2
    \$\begingroup\$ Also, if you have Python 3, they already included regex as re, for free! What a deal! \$\endgroup\$ – BruceWayne Oct 4 '19 at 15:33
8
\$\begingroup\$

Return Simplification

This

def is_punctuation(char):
    return True if unicodedata.category(char).startswith("P") else False

can be

def is_punctuation(char):
    return unicodedata.category(char).startswith("P")

Since unicodedata.category(char).startswith("P") evaluates to a boolean expression, you can return the expression.

Type Hints

These function headers

def split_on_punc(text):
def is_punctuation(char):

can be this

def split_on_punc(text: str) -> list:
def is_punctuation(char: str) -> bool:

Type Hints allow you to show what types of parameters are supposed to be passed, and what types are returned by these functions.

Docstrings

You should include a docstring at the beginning of every function, class and module you write. This will allow you to describe what functions do what, and what modules contain what functions and their purpose.

\$\endgroup\$
  • \$\begingroup\$ I think that the chr type hint is not correct, but I won't know for sure until this is answered: stackoverflow.com/questions/58229400/… \$\endgroup\$ – Reinderien Oct 4 '19 at 3:07
  • \$\begingroup\$ @Reinderien The python docs show that unicodedata.category takes a chr, from what I interpreted. \$\endgroup\$ – Linny Oct 4 '19 at 3:08
  • 1
    \$\begingroup\$ Those chr are variable names, not type hints. \$\endgroup\$ – Reinderien Oct 4 '19 at 3:09
  • \$\begingroup\$ chr is not a valid type. \$\endgroup\$ – juanpa.arrivillaga Oct 4 '19 at 3:11
  • 1
    \$\begingroup\$ @Reinderien That makes sense. Reading the comments on your stackoverflow post also point to the idea of using str. I will edit accordingly. \$\endgroup\$ – Linny Oct 4 '19 at 3:12
3
\$\begingroup\$

First things:

You can trim off a little just by making the loop tighter. Python does pretty cool "for" loops:

for char in chars will loop through every character in the list 'chars'. That means you don't need to deal with the variable 'i'. In fact, you don't even need to cast the input to a list called 'chars'. Python can iterate through a string as if it were a list. So your loop can be:

for char in text

By my count, that saves you four lines of code right there.

-

Next: Use fewer cases:

As written, you've effectively got three relevant paths through the loop, for three possible cases:

  1. The character is punctuation.
  2. The character isn't punctuation, and it's the first character after punctuation
  3. The character isn't punctuation, and it's not the first character after punctuation

The distinction between (2) and (3) doesn't depend on the character you're dealing with. It depends on the previous character, which is why you've had to use the "start_new_word" variable to carry information over from one iteration of the loop into the next. I think loops are easier to deal with if you don't have to carry information over like that. So I'd want to find a way to eliminate it.

You're effectively using "start_new_word" as a signal that tells you to do output.append([]), at the start of the next loop. But why wait until the next loop? If you replace the start_new_word = true instructions with output.append([]) instructions, then you'll always have a clean 'word' sitting at the end of the output list, ready to accept characters. That way, whenever you have a non-punctuation character you just append it to output[-1], and you don't have to worry about whether it's the first character.

\$\endgroup\$
  • \$\begingroup\$ One snag: if you have two punctuation characters in a row, then this method will put an empty word in between them. [ "text", "@", "", "?"]. - After you add an 'if' statement to deal with that, the function ends up just as long as it was before. But I think it's still 'cleaner' that way, without having to carry information over between loops. \$\endgroup\$ – Douglas Winship Oct 4 '19 at 13:50
  • \$\begingroup\$ Also, you're working with lists of characters, and then converting them to strings at the end with a join method. You could just work directly with strings. Instead of output.append([]) and output[-1].append(new_character) you can do output.append('') and output[-1] += new_character That saves you from having to do the .join at the end. \$\endgroup\$ – Douglas Winship Oct 4 '19 at 13:57
2
\$\begingroup\$

This kind of function could be designated for an API and should be clear for every developer that would use it.

As you probably know , there is already a split method in string object. This method doesn't return separators (as they are usually useless).

For example:

"this.is.codereview".split('.')

outputs:

['this', 'is', 'codereview']

The naming of split_on_punc could be modified to spotlight this difference, or at least be clarified with a docstring.

I'll advise you to use explicit name of category in is_ponctuation to avoid ambiguities and unwilled split if another Category starts with 'P'.

def is_punctuation(char):
    return unicodedata.category(char) == "Po"

This function (inspired from this answer) does quite the trick if you doesn't need to be too strict on ponctuation separators.

import string
import re

def split_on_punc(text):
    return [
      token
      for token in re.split("\(W)", text)
      if token not in string.whitespace
    ]

It splits the string considering every non-alphanumeric character as ponctuation using split from re standard python library then removes whitespaces from list. It's a way far less precise than Austin answer.

\$\endgroup\$
  • \$\begingroup\$ Hey, good first answer, welcome on CodeReview :) \$\endgroup\$ – IEatBagels Oct 4 '19 at 17:09
0
\$\begingroup\$

Depending on how exactly you want to output the punctuation, possibilities range from the one-liner

def split_merging_punctuation(line):
    return ("".join(cs) for _, cs in itertools.groupby(line, is_punctuation))

to a function that produces output equivalent to yours:

def split_not_merging_punctuation(line):
    for is_p, chars in itertools.groupby(line, is_punctuation):
        if is_p:
            yield from chars
        else:
            yield "".join(chars)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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