Speed up preprocessing module

[mk2510]

Looked at every function in the preprocessing module 😑 The only function we found, where it makes sense to speed up was the clean function 🧹 with the default pipeline. This is probably called with the default pipeline very often. The default pipeline loops through every function and we combined these into one pass in _optimised_default_clean_single_cell(text: str), which is 30% faster than the default pipeline. Now when users call clean with the default pipeline, this function will be used.
We understand, that this introduces code duality and more maintenance if bugs were fixed and changes to the default pipeline introduced, but we believe it makes sense, as the function is called so often. 🦝

[jbesomi]

Nice, thank you! 🎉

Review:

1. What about get_default_pipeline? We can either keep it (but why?), remove it or rename _optimised_default_clean -> get_default_pipeline?
2. We need to extensively unit-test it as this will be one of the most used function
3. ... what if we have global constants regex?
4. What if we update the docstring of clean explaining the new changes?

[mk2510]

Review:
1.What about get_default_pipeline? We can either keep it (but why?), remove it or rename _optimised_default_clean -> get_default_pipeline?

2. We need to extensively unit-test it as this will be one of the most used function
3. what if we have global constants regex?
   4.What if we update the docstring of clean explaining the new changes?

I have just updated the files to include those changes. 😅

[jbesomi]

Probably, the words REMOVE, REPLACE and PATTERN_ are not necessary:

PATTERN_REMOVE_DIGITS_BLOCK -> DIGITS_BLOCK
PATERN_REMOVE_CURLY_BRACKETS -> CURLY_BRACKETS
...

[jbesomi]

Global functions are always lowercased
"Returns the standart tokenisation pattern": not particularly meaningful
"Returns" -> "Return"

[mk2510]

ah thanks 🥇 I was unsure about the python style guide. But now it is a private function

[mk2510]

I just have implemented those changes. Thx for the comments on python style guides 👍
Also fixed some bigger merge problems in the preprocessing module

[jbesomi]

The new clean solution makes use of apply. But, in some cases (for instance with remove punctuation) we might not need to use apply, rather prefer s.str.replace(REGEX), no?

I'm OK in improving the clean function but first I think we need to have a better understanding of which solution is actually the fastest, i.e we need to benchmark a bit.

It would be great if you can do some benchmark and prepare some reports/insights, maybe as a single Jupiter notebook that you can attach here.

A possible benchmark pipeline might be:

1. apply(re.sub) vs str.replace
2. your version vs old version
3. go even further and try to parallelize more (with flashtext flashtext integration #87 for instance?)

We can also split this PR in two:

1. Add REGEX constants
2. Benchmark clean + add clean + add unit tests

[jbesomi]

word standart does not mean anything for most of the users ...
we should come up with a better explanation; i.e mentioning both what does this function does and why, which problem it solves

[mk2510]

I have updated the comment. I hope it will be clearer now 🐰

[jbesomi]

Not sure we really need so many tests for this part ...?

[mk2510]

I think those test will help us to cover all different sections of the pipeline individually, so if something gets changed, we know, which part is broken

[mk2510]

apply(re.sub) vs str.replace
your version vs old version

@jbesomi I have created here a notebook https://colab.research.google.com/drive/1HeVzomOS2F962qxTxKu_W__e1HaIfvZq?usp=sharing which presents the time differences.

go even further and try to parallelize more (with flashtext #87 for instance?)

as far, as I understood does the linked pull bring fasttext to our library. In my experience fasttext will be faster with huge regex pattern and not with the small ones we still have. So I think, this integration might be more part of the other issue 🏎️

@henrifroese and I will start with the multithreading task today, which will give general improvements. This pull request should just improve the functions by more efficient code 🤓

[mk2510]

apply(re.sub) vs str.replace

we need to do the re.sub as str.replace does not take any regex expressions when performed on a string.

[henrifroese]

After doing more speed comparisons here, we have noticed that

• the new clean function is actually not faster after all -> don't change it
• str.replace is as fast as apply(re.sub) -> don't change it

So we're closing 🚪 this 🦀 🌵 😿