Porting stemming algorithm from Java to Python

Question

I have ported Stempel stemmer in Java (Apache Lucene) to Python. I come from Java world, so I'm afraid my translation might not be "pythonic" enough.

I would like to hear your feedback on quite representative part of the code, translation of Diff class that applies stemming command (diff) to a string (dest).

 @classmethod
 def apply(cls, dest, diff):
 """
 Apply the given patch string diff to the given string dest
 :param dest: Destination string
 :param diff: Patch string
 :return:
 """
 if diff is None:
 return
 if not isinstance(dest, MutableString):
 raise ValueError
 if not dest:
 return
 pos = len(dest) - 1
 try:
 for i in range(int(len(diff) / 2)):
 cmd = diff[2 * i]
 param = diff[2 * i + 1]
 par_num = ord(param) - ord('a') + 1
 if cmd == '-':
 pos -= (par_num - 1)
 elif cmd == 'R':
 cls.__check_index(dest, pos)
 dest[pos] = param
 elif cmd == 'D':
 o = pos
 pos -= (par_num - 1)
 cls.__check_index(dest, pos)
 dest[pos:o + 1] = ''
 elif cmd == 'I':
 pos += 1
 cls.__check_offset(dest, pos)
 dest.insert(pos, param)
 pos -= 1
 except IndexError:
 # swallow, same thing happens in original Java version
 pass
@classmethod
def __check_index(cls, s, index):
 if index < 0 or index >= len(s):
 raise IndexError
@classmethod
def __check_offset(cls, s, offset):
 if offset < 0 or offset > len(s):
 raise IndexError

Some justifications of decisions I took:

• The original implementation uses StringBuffer to manipulate characters in a string. Since Python str type is immutable I used my own class MutableString that behaves like a Python list.

• Also, original logic was based on catching IndexOutOfBoundsException. Contrary to Java, Python allows negative indexes in a list and list ranges. Therefore, I've introduces guards like __check_X.

• Java implementation uses switch/case/default clause. I translated that to if/elif/else clause in Python.

Answer 1

Specific suggestions:

1. Python will throw an IndexError if you try to access an element past the end of a list, so you don't need to check index >= len(s) or offset > len(s).
2. After the simplification above I would inline the __check methods as assertions.
3. Throwing an exception and catching it again within the same context is a code smell - it's too much like goto. Why not just return instead? Or let the user know the input could not be processed?
4. You don't actually use any OOP in your code, so apply might as well not be within a class
5. In the same way I think you could just use a List[str] for dest.
6. Checking the type of an argument is unpythonic. Basically you're responsible for passing some value which can be used by the code. And with type checking (below) you could even enforce this at a test or linting stage.
7. Abbreviations make code harder to read. I would expand things like destination, position and command.
8. Magic values like R and D should be named constants to improve readability.

General suggestions:

1. black can automatically format your code to be more idiomatic.
2. isort can group and sort your imports automatically.

3. flake8 with a strict complexity limit will give you more hints to write idiomatic Python:

   [flake8]
   max-complexity = 4
   ignore = W503,E203
   

   That limit is not absolute by any means, but it's worth thinking hard whether you can keep it low whenever validation fails. For example, I'm working with a team on an application since a year now, and our complexity limit is up to 7 in only one place. Conversely, on an ugly old piece of code I wrote without static analysis support I recently found the complexity reaches 87!)

4. I would then recommend adding type hints everywhere and validating them using a strict mypy configuration:

   [mypy]
   check_untyped_defs = true
   disallow_untyped_defs = true
   ignore_missing_imports = true
   no_implicit_optional = true
   warn_redundant_casts = true
   warn_return_any = true
   warn_unused_ignores = true
   

   As a Java developer I'm sure you'd appreciate the clarity this lends to the code.

• dzieciou: Thanks. I've started to introduce type hinting but I've realized in many cases it makes reading the code harder. Right, it helps clarify ambiguous parts but in other parts it's seems overly verbose like in Java. What's your take on using type hinting selectively, for documentation purposes only when the code is ambiguous?
• l0b0: Seems sensible if any of the types get complicated. For this algorithm I expect the finished code would make for relatively simple type hints.