Combine special characters into single token

[loostrum]

Fixes #437 . See that issue for more details on why the tokenizer was updated as a fix.

[cwmeijer]

I ran the tests locally and they pass of course.

Only when using the dashboard, I still got some 'Could not create tensor from given input list' errors. For instance:
let'''s watch this together
review with !!!?
review with! ?
review with???!
such a bad movie "!?\'" particularly strange one, as it is from your (passing) unit test

These work fine though:
review with !!!
review with!?
review with???

I also tried some of these by substituting the current text in the unit test with the texts above. There they seem to be processed just fine.

Could you take another look at the behavior in the dashboard?

[loostrum]

Thanks for checking the dashboard! I'm looking at it now, I see the dashboard actually uses the Spacy tokenizer directly instead of the tokenizer class we defined in DIANNA. I'll try to fix that.

[loostrum]

All those cases should pass now (but please check if you have time), except for let'''s watch this together.
That is a particularly tricky one, because the special characters are in the middle of a word. This is how Spacy tokenizes that plus a version where the middle ' is masked:

In [9]: spacy("let'''s")
Out[9]: ['let', "''", "'s"]

In [10]: spacy("let'UNKNOWNWORD's")
Out[10]: ["let'UNKNOWNWORD", "'s"]

The special character fix doesn't handle this, because Spacy for some reason doesn't create a new token starting at the first ' in the second case.

I'm not sure if/how we can fix this. For now I would say that the behaviour is already much improved and hopefully the still breaking cases don't really occur in the wild anyway.

[loostrum]

Hmm weird error in the CI, seems unrelated to the PR: https://github.com/dianna-ai/dianna/actions/runs/4184062075/jobs/7249224208

[cwmeijer]

Great improvement indeed.
We talked about adding a list of extra tests of all strange cases that we cover now. Those would be great still.
We should also create an issue for the remaining cases that fail, even if they are very unlikely cases.

[cwmeijer]

I don't know why importing onnx in python 3.9 under macos gives problems. It happens outside this branch as well.

[elboyran]

It also has a conflict.
Earlier I've merged an approved PR from Leon ( #463 ) and then the error started, but don't know how to fix it.

[cwmeijer]

After merging main into this and solving a single conflict, which seemed straight forward, lime is giving explanations omitting the special characters. I will look into this later.

[cwmeijer]

There is still something wrong with running the notebooks. We should see if it's an actual bug. If so, we should write tests so the actual tests are failing as well. Right now it looks like everything is just fine except for the notebooks (which is probably not the case).

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[cwmeijer]

The linting is failing on import order of time series visualization. This has nothing to do with this PR and has already been fixed in main (but after triggering the actions for this PR).

[cpranav93]

It looks good! Go ahead and merge.