Hi thanks for cool crate!

Could you remove or make to_ascii_lowercase optional? I think such pre-processing should be done on the library client side, since it is simple (.map(|doc| doc.to_ascii_lowercase())), and is not required for main heavy tokenization fitting and transform logic, I would prefer to call it my self when needed.

@rth awesome, I'm all about collaboration; I'm going to checkout the package after work today!

Great, thank you @jbowles! Feel free to write any general comments you have about this project here (or in any of the related issues).

To give you some background, I have been working on topics related to CountVectorizer / HashingVectorizer in scikit-learn for a few years and this project originated as an attempt in making those faster. A few things got added along the way. I'm a fairly beginner Rust programmer, so general feedback about the architecture of this crate would be very welcome. In particular, adding more common traits per module would probably be good (I started some of the work on it in #48). Some of it was also limited by the fact that I wanted to make a thin wrapper in PyO3 to expose the functionality in Python which adds some constraints (e.g. #48 (comment))

For tokenization, one thing I saw was that if one takes the unicode-segmentation crate, it will tokenize the text almost exactly as expected for NLP applications, with a few exceptions. The nice thing about it is that it's language independent and based on the Unicode spec, which removes the need to maintain a large number of regexp / custom rules. To improve the F1 score for tokenization on the UD treebank a few custom rules are additionally applied.

On the other side, we can imagine other tokenizers. In particular, the fact that some tasks require custom processing is a valid point. I'm not sure how to make that easier.

I also found an implementation of punkt tokenizer rust-punkt!

Yes, it looks quite good. Related issue #51

Generally if can do anything to make this collaboration easier please let me know :)

Once #1 is merged, it would be good to build binary Python wheels and upload them on PyPi.

setuptools-rust sounds like the most simple solution.

• Linux
• MacOS
• Windows

Currently, we use the MurmurHash3 hash function from the rust-fasthash (to be more similar to scikit-learn implementation). That crate also supports a number of other hash functions,

City Hash
Farm Hash
Metro Hash
Mum Hash
Sea Hash
Spooky Hash
T1 Hash
xx Hash

I'm not convinced hashing is currently the performance bottleneck, but in any case using a faster hash function such as xxhash would not hurt.

This would involve updating the text-vectorize crate and adding hasher parameter to the HashingVectorizer python estimator.

Another use case could to use different hash functions to reduce the effect of collisions Svenstrup et. al. 2017, discussed e.g. in https://stackoverflow.com/q/53767469/1791279

In order to use the python package as a drop in replacements of scikit-learn's text vectorizers we would need,

• Wrap HashingVectorizer #1
• Wrap CountVectorizer #14
• Implement most common functionality
• Pass scikit-learn tests (when they make sense).

Allowing tokenize documents with character n-grams would be useful.

It would be necessary to implement IDF transforms, and possibly expose a TfidfVectorizer estimator.

This requires selecting a sparse array library. For now, we use custom CSRArray structs to represent CSR arrays. https://github.com/vbarrielle/sprs is a good candidate but this needs more investigation in any case.

Link time Optimization (LTO) would add around 10% in performance for vtext and also reduce binary sizes. However, it also increases the compilation/link time significantly so we may want to only enable it for commits with an existing release tag.

Currently, tokenizers return Vec<String> where tokens are be slices of the input string. Moving to Vec<&str> would remove one memory copy and is likely to help with run time.

This should be possible with PyO3 0.7.0 (not yet released) that will allow using lifetime specifiers in pymethods.

Currently CountVectorizer and HashingVectorizer mostly perform BOW token counting without the possibility to change the tokenizer or any other parameters.

While we intentionally won't support all the parameter that scikit-learn versions does (as these meta-estimators are doing too much), additional parametrization would be preferable.

• parametrization of the tokenizer will be addressed in #48

It can happen that the tokenization results are unsatisfactory in some way, and the question is what should be the mechanism to customize/improve them. Whether it should be by,
a) adding options make these optional improvements in the tokenizer. The issue with these is that some of these might be relevant to multiple tokenizers
b) add a new step later in the pipeline. That's probably the best way to allow arbitrary customization. The issue is that some steps might be specific to the previous step, and adding them in the library might be confusing.

There is probably a balance that needs to be found between the two.

For instance,

1. PunctuationTokenizer,
   • currently doesn't take into account repeated punctuation

     >>> PunctuationTokenizer().tokenize("test!!!")                                                                                                     
     ['test!', '!', '!']

   • will tokenize abbreviations separated by . as separate sentence

     >>> PunctuationTokenizer().tokenize("W.T.O.")
     ['W.', 'T.', 'O.']

   both could probably be addressed by adding an option to force sentences to be longer than some minimal length (and otherwise append them to the previous token).
2. UnicodeSentenceTokenizer,
   will not tokenizer sentences separated by a punctuation without space e.g.,

   >>> UnicodeSentenceTokenizer().tokenize('One sentence.Another sentence.')
   ['One sentence.Another sentence.']

   That's a very common occurrence in actual text, and I think a workaround should be found (e.g. using an additional tokenization pass with a regex/punctuation tokenizer).

Generally it would be good to add some evaluation benchmarks to evaluation/ for sentence tokenization to evaluation/ folder.

3. UnicodeTokenizer is currently extended in VTextTokenizer (for lack of a better name), with a few additional rules. Maybe this could have been a separate token-processing step, particularly if one imagine that more rules could be added (or potentially even using an ML model).

Currently, only bag of word vectorization is implemented. It would be good to extend the code to word n-grams as well.

UnicodeSegmentTokenizer was meant to be a shorter version of Unicode segmentation tokenizer, but the name is not very explicit. Besides UnicodeSentenceTokenizer also uses unicode-segmentation crate which adds to the confusion. Maybe UnicodeWordTokenizer would be a better name?

From #78 (comment) by @joshlk

I think it would be sensible to identify different languages throughout the package using ISO two-letter codes (e.g. en, fr, de ...).

In particular, we should implement this for the Snowball stemmer in python which currently uses the full language names.

I am also wondering if in Rust, we should use String for the language parameter or define an Enum e.g.

use vtext::lang

let stemmer = SnowballStemmerParams::default().lang(lang::en).build()

The latter is probably simpler, but it makes it a bit harder to extend e.g. if someone designs an custom estimator for a language not in the list (e.g. some ancient infrequently used language), they would have to create a new enum.

Also just to be consistent the parameter name would be "lang" not "language", right?

Currently, Python classes / functions generated with Pyo3 are not picklable (PyO3/pyo3#100) which makes their use problematic in typical data science workflows (e.g. with joblib parallel or in scikit-learn pipelines).

Implementing __getstate__, __setstate__ methods is probably necessary to make it work.

Ideally, an NLP pipeline in Rust could look something like,

preprocessor = DefaultPreprocessor::new()
tokenizer = RegexpTokenizer::new(r"\b\w\w+\b")
stemmer = SnowballStemmer::new("en")
analyzer = NgramAnalyzer(range=(1, 1))

pipe = collection
          .map(preprocessor)
          .map(tokenizer)
          .map(|tokens| tokens.map(stemmer))
          .map(analyzer)

where collection is an iterator over documents.

There are several chalenges with it though,

• It is better to avoid allocating strings for tokens in each pre-processing step and instead use a slice of the original document. Performance depends very strongly on this. The current implementation e.g. of RegexpTokenizer takes a reference to the document and return an Iterable of &str with the same lifetime as the input document, but then borrow checker doesn't appear to be happy when it is used in the pipeline. This may be related to using closures (cf next point) though.
• Because structs are not callable, collection.map(tokenizer) doesn't work,
  nor does collection.map(tokenizer.tokenize) (i.e. using a method) for some reason. We can use collection.map(|document| tokenizer.tokenize(&document)) but then lifetime is not properly handled between input and output (described in the previous point).

More investigation would be necessary, and both points are likely related.

It would be useful to add a sentence splitter, for instance, possibilities could be,

• Puntk sentence tokenizer from NLTK (needs pre-trained model)
• Unicode sentence boundaries from unicode-rs/unicode-segmentation#24 (doesn't need a pre-trained model)
• investigate spacy implementation (likely needs pre-trained model)

Currently, the VTextTokenizer first computes Unicode segmentation (which should handle Unicode well by definition) than applies a few simple rules on top to produce tokenization that is more standard in NLP (and possibly language dependent).

These rules might need to be generalized a bit to handle Unicode better. For instance, currently we merge tokens linked by - but only the ascii one, not on other Unicode variants.