Repository: adbar/simplemma

Issue:
Broken dictionary building
Replacing the language data with the `training/dictionary_pickler.py` script completely degrades, the previous results cannot be replicated anymore.

I believe that commit 63933fc accidentally reverted the conversion to bytestrings shortly before in 546ab7e, so that the lemmatizer ends up looking for binary data that is not there, which does not throw an error but does not return anything useful either.

The crucial line is here:
`return {k.encode("utf-8"): v.encode("utf-8") for k, v in sorted(mydict.items())}`
https://github.com/adbar/simplemma/commit/63933fcfd2e4fa9bb1626fa959d9777d9dee5f57#diff-f5e1d8737ff169b2b390ac79e7292d17bc3a41c2c2de3e6f2b348c78a70af724R123

@Dunedan It was your PR, could you please have a look at it? We should at best integrate the previous state into your code.

Discussion:
@Dunedan: > I believe that commit 63933fc accidentally reverted the conversion to bytestrings shortly before in 546ab7e, so that the lemmatizer ends up looking for binary data that is not there, which does not throw an error but does not return anything useful either.

That wasn't accidentally, but intentionally. @juanjoDiaz introduced that idea in #135 and I incorporated it in #133 after we discussed it (see https://github.com/adbar/simplemma/pull/133#issuecomment-2159178454 and the following comments).
In fact, the changes regarding the bytestrings were even a separate commit in #133, but got merged by you into a single one, when you merged the PR. The commit message nevertheless states these changes and their intention as well.

> Replacing the language data with the `training/dictionary_pickler.py` script completely degrades, the previous results cannot be replicated anymore.

I reverted the changes made regarding the bytestrings to `training/dictionary_pickler.py` in 63933fc, while I shouldn't have done that. Here is a PR to fix it: #156@adbar: Thanks for the details and the quick response, it's really helpful.

Solution patch:
diff --git a/training/dictionary_pickler.py b/training/dictionary_pickler.py
index 69f4692..15345d1 100644
--- a/training/dictionary_pickler.py
+++ b/training/dictionary_pickler.py
@@ -10,7 +10,7 @@
 import re
 from operator import itemgetter
 from pathlib import Path
-from typing import Dict, List, Optional
+from typing import ByteString, Dict, List, Optional
 
 import simplemma
 from simplemma.strategies.defaultrules import DEFAULT_RULES
@@ -49,7 +49,9 @@ def _determine_path(listpath: str, langcode: str) -> str:
     return str(Path(__file__).parent / filename)
 
 
-def _read_dict(filepath: str, langcode: str, silent: bool) -> Dict[str, str]:
+def _read_dict(
+    filepath: str, langcode: str, silent: bool
+) -> Dict[ByteString, ByteString]:
     mydict: Dict[str, str] = {}
     myadditions: List[str] = []
     i: int = 0
@@ -120,12 +122,12 @@ def _read_dict(filepath: str, langcode: str, silent: bool) -> Dict[str, str]:
         mydict[word] = word
     LOGGER.debug("%s %s", langcode, i)
     # sort and convert to bytestrings
-    return dict(sorted(mydict.items()))
+    return {k.encode("utf-8"): v.encode("utf-8") for k, v in sorted(mydict.items())}
 
 
 def _load_dict(
     langcode: str, listpath: str = "lists", silent: bool = True
-) -> Dict[str, str]:
+) -> Dict[ByteString, ByteString]:
     filepath = _determine_path(listpath, langcode)
     return _read_dict(filepath, langcode, silent)
 


Test patch:
diff --git a/tests/test_dictionary_pickler.py b/tests/test_dictionary_pickler.py
index 37136f2..2fc806f 100644
--- a/tests/test_dictionary_pickler.py
+++ b/tests/test_dictionary_pickler.py
@@ -26,9 +26,9 @@ def test_logic() -> None:
     # different order
     mydict = dictionary_pickler._read_dict(testfile, "es", silent=True)
     assert len(mydict) == 5
-    assert mydict["closeones"] == "closeone"
+    assert mydict[b"closeones"] == b"closeone"
     item = sorted(mydict.keys(), reverse=True)[0]
-    assert item == "valid-word"
+    assert item == b"valid-word"
 
     # file I/O
     assert dictionary_pickler._determine_path("lists", "de").endswith("de.txt")