Repository: bootphon/phonemizer (language: python)

Issue:
The punctuation mark is not separated correctly when using space as the phone separator in v0.3.1
Thank you for developing a great tool.
This is very useful to develop our speech processing toolkit.
I found unexpected behavior in v0.3.1.

**Describe the bug**
The punctuation mark is not separated correctly when using space as the phone separator.

**Phonemizer version**
- 0.3.1

**System**

- OS information: `Linux 3.10.0-1062.9.1.el7.x86_64 #1 SMP Mon Dec 2 08:31:54 EST 2019 x86_64`
- python version: `3.9.7 (default, Sep 16 2021, 13:09:58)  [GCC 7.5.0]`

**To reproduce**
```python
from phonemizer import phonemize
from phonemizer.separator import Separator

text = "Hello, world."
separator = Separator(
    word=None,
    syllable=None,
    phone=" ",
)
phn = phonemize(text, separator=separator, preserve_punctuation=True)
print(phn)
```

**Expected behavior**
In phonemizer==0.3 (expected)
```bash
$ python test_phonemizer.py
h ə l oʊ , w ɜː l d .
```

In phonemizer==0.3.1 (seems wrong)
```bash
$ python test_phonemizer.py
h ə l oʊ, w ɜː l d.
```


Maximum recursion depth exceeded with --preserve-punctuation and long documents
**Describe the bug**
If you try to phonemize a long document (say, a Project Gutenberg ebook) and preserve punctuation, phonemize throws
`fatal error: maximum recursion depth exceeded in comparison`

**Phonemizer version**
phonemizer-3.0.1
available backends: espeak-1.48.3, segments-2.2.0
uninstalled backends: espeak-mbrola, festival

**System**
Both macOS 11.6 and Ubuntu 20.10

**To reproduce**
`phonemize --preserve-punctuation pg67147.txt`
where pg67147.txt is [this ebook](https://www.gutenberg.org/ebooks/67147), or anything relatively long.

**Expected behavior**
This looks like a result of the Punctuation restore methods being recursive and my (probably unreasonably long) use case. 

**Additional context**
I'm happy to try refactoring the `restore` methods into a single iterative method. Just let me know if you'd like me to contribute. 



Discussion / hints:

Please open PR, I've got the same issue and your refactoring seems to fix it.
My previous PR for this project (#103, to add a simple new feature) is still pending, and I did my refactoring of the punctuation restore method on a branch off of that work. My git abilities are kind of rusty, so I'm worried that I'd mess something up if I tried to make a new PR independent from my other one. So hopefully the maintainers will review my first PR soon and then I can proceed with my fix for this.

Fix patch:
diff --git a/phonemizer/backend/base.py b/phonemizer/backend/base.py
index 44771c4..ce132c3 100644
--- a/phonemizer/backend/base.py
+++ b/phonemizer/backend/base.py
@@ -195,7 +195,7 @@ def phonemize(self, text: List[str],
             # flatten them in a single list
             phonemized = self._flatten(phonemized)
 
-        return self._phonemize_postprocess(phonemized, punctuation_marks)
+        return self._phonemize_postprocess(phonemized, punctuation_marks, separator, strip)
 
     @staticmethod
     def _flatten(phonemized: List[List[Any]]):
@@ -232,12 +232,15 @@ def _phonemize_preprocess(self, text: List[str]) -> Tuple[Union[str, List[str]],
             return self._punctuator.preserve(text)
         return self._punctuator.remove(text), []
 
-    def _phonemize_postprocess(self, phonemized: List[str], punctuation_marks):
+    def _phonemize_postprocess(self, phonemized: List[str],
+                               punctuation_marks,
+                               separator: Separator,
+                               strip: bool):
         """Postprocess the raw phonemized output
 
         Restores the punctuation as needed.
 
         """
         if self._preserve_punctuation:
-            return self._punctuator.restore(phonemized, punctuation_marks)
+            return self._punctuator.restore(phonemized, punctuation_marks, separator, strip)
         return phonemized
diff --git a/phonemizer/backend/espeak/espeak.py b/phonemizer/backend/espeak/espeak.py
index bd1f8c1..1ebf4b6 100644
--- a/phonemizer/backend/espeak/espeak.py
+++ b/phonemizer/backend/espeak/espeak.py
@@ -146,14 +146,14 @@ def _phonemize_preprocess(self, text: List[str]) -> Tuple[Union[str, List[str]],
         self._words_mismatch.count_text(text)
         return text, punctuation_marks
 
-    def _phonemize_postprocess(self, phonemized, punctuation_marks):
+    def _phonemize_postprocess(self, phonemized, punctuation_marks, separator: Separator, strip: bool):
         text = phonemized[0]
         switches = phonemized[1]
 
         self._words_mismatch.count_phonemized(text)
         self._lang_switch.warning(switches)
 
-        phonemized = super()._phonemize_postprocess(text, punctuation_marks)
+        phonemized = super()._phonemize_postprocess(text, punctuation_marks, separator, strip)
         return self._words_mismatch.process(phonemized)
 
     @staticmethod
diff --git a/phonemizer/punctuation.py b/phonemizer/punctuation.py
index 5468d4a..5c1dddc 100644
--- a/phonemizer/punctuation.py
+++ b/phonemizer/punctuation.py
@@ -19,6 +19,7 @@
 from typing import List, Union, Tuple
 
 from phonemizer.utils import str2list
+from phonemizer.separator import Separator
 
 # The punctuation marks considered by default.
 _DEFAULT_MARKS = ';:,.!?¡¿—…"«»“”'
@@ -136,54 +137,77 @@ def _preserve_line(self, line: str, num: int) -> Tuple[List[str], List[_MarkInde
         return preserved_line + [line], marks
 
     @classmethod
-    def restore(cls, text: Union[str, List[str]], marks: List[_MarkIndex]) -> List[str]:
+    def restore(cls, text: Union[str, List[str]],
+                marks: List[_MarkIndex],
+                sep: Separator,
+                strip: bool) -> List[str]:
         """Restore punctuation in a text.
 
         This is the reverse operation of Punctuation.preserve(). It takes a
-        list of punctuated chunks and a list of punctuation marks. It returns a
-        a punctuated text as a list:
+        list of punctuated chunks and a list of punctuation marks, as well as
+        the separator and strip parameters used by phonemize. It returns the
+        punctuated text as a list:
 
             ['hello', 'my world'], [',', '!'] -> ['hello, my world!']
 
         """
-        return cls._restore_aux(str2list(text), marks, 0)
-
-    @classmethod
-    def _restore_current(cls, current: _MarkIndex, text: List[str], marks: List[_MarkIndex], num) -> List[str]:
-        """Auxiliary method for Punctuation._restore_aux()"""
-        text[0] = text[0].rstrip()
-        if current.position == 'B':
-            return cls._restore_aux(
-                [current.mark + text[0]] + text[1:], marks[1:], num)
-
-        if current.position == 'E':
-            return [text[0] + current.mark] + cls._restore_aux(
-                text[1:], marks[1:], num + 1)
-
-        if current.position == 'A':
-            return [current.mark] + cls._restore_aux(text, marks[1:], num + 1)
-
-        # position == 'I'
-        if len(text) == 1:  # pragma: nocover
-            # a corner case where the final part of an intermediate
-            # mark (I) has not been phonemized
-            return cls._restore_aux([text[0] + current.mark], marks[1:], num)
-
-        return cls._restore_aux(
-            [text[0] + current.mark + text[1]] + text[2:], marks[1:], num)
-
-    @classmethod
-    def _restore_aux(cls, text: List[str], marks: List[_MarkIndex], num: int) -> List[str]:
-        """Auxiliary method for Punctuation.restore()"""
-        if not marks:
-            return text
-
-        # nothing have been phonemized, returns the marks alone
-        if not text:
-            return [''.join(m.mark for m in marks)]
-
-        current = marks[0]
-        if current.index == num:  # place the current mark here
-            return cls._restore_current(current, text, marks, num)
-
-        return [text[0]] + cls._restore_aux(text[1:], marks, num + 1)
+        text = str2list(text)
+        punctuated_text = []
+        pos = 0
+
+        while text or marks:
+
+            if not marks:
+                merged_text = ''.join(text)
+                # if strip is False, ensure the final word ends with a word separator
+                if not strip and sep.word and not merged_text.endswith(sep.word):
+                    merged_text = merged_text + sep.word
+                punctuated_text.append(merged_text)
+                text = []
+            elif not text:
+                # nothing has been phonemized, returns the marks alone, with internal
+                # spaces replaced by the word separator
+                punctuated_text.append(re.sub(' ', sep.word, ''.join(m.mark for m in marks)))
+                marks = []
+
+            else:
+                current_mark = marks[0]
+                if current_mark.index == pos:
+
+                    # place the current mark here
+                    mark = marks[0]
+                    marks = marks[1:]
+                    # replace internal spaces in the current mark with the word separator
+                    mark = re.sub(' ', sep.word, mark.mark)
+
+                    # remove the word last separator from the current word
+                    if sep.word and text[0].endswith(sep.word):
+                        text[0] = text[0][:-len(sep.word)]
+
+                    if current_mark.position == 'B':
+                        text[0] = mark + text[0]
+                    elif current_mark.position == 'E':
+                        punctuated_text.append(text[0] + mark + ('' if strip or mark.endswith(sep.word) else sep.word))
+                        text = text[1:]
+                        pos = pos + 1
+                    elif current_mark.position == 'A':
+                        punctuated_text.append(mark + ('' if strip or mark.endswith(sep.word) else sep.word))
+                        pos = pos + 1
+                    else:
+                        # position == 'I'
+                        if len(text) == 1:  # pragma: nocover
+                            # a corner case where the final part of an intermediate
+                            # mark (I) has not been phonemized
+                            text[0] = text[0] + mark
+                        else:
+                            first_word = text[0]
+                            text = text[1:]
+                            text[0] = first_word + mark + text[0]
+
+                else:
+                    punctuated_text.append(text[0])
+                    text = text[1:]
+                    pos = pos + 1
+
+
+        return punctuated_text


Test patch:
diff --git a/test/test_phonemize.py b/test/test_phonemize.py
index d156c9e..3e0e6fd 100644
--- a/test/test_phonemize.py
+++ b/test/test_phonemize.py
@@ -194,73 +194,73 @@ def test_segments(njobs):
             ['həloʊ wɜːld ', 'ɡʊdbaɪ ']),
         ('espeak', False, True, False,
             ['hello world!', '', 'goodbye'],
-            ['həloʊ wɜːld!', 'ɡʊdbaɪ ']),
+            ['həloʊ wɜːld! ', 'ɡʊdbaɪ ']),
         ('espeak', True, False, False,
             ['hello world!', '', 'goodbye'],
             ['həloʊ wɜːld ', '', 'ɡʊdbaɪ ']),
         ('espeak', True, True, False,
             ['hello world!', '', 'goodbye'],
-            ['həloʊ wɜːld!', '', 'ɡʊdbaɪ ']),
+            ['həloʊ wɜːld! ', '', 'ɡʊdbaɪ ']),
         ('segments', False, False, False,
             ['achi acho?', '', 'achi acho'],
             [u'ʌtʃɪ ʌtʃʊ ', u'ʌtʃɪ ʌtʃʊ ']),
         ('segments', False, True, False,
             ['achi acho?', '', 'achi acho'],
-            [u'ʌtʃɪ ʌtʃʊ?', u'ʌtʃɪ ʌtʃʊ ']),
+            [u'ʌtʃɪ ʌtʃʊ? ', u'ʌtʃɪ ʌtʃʊ ']),
         ('segments', True, False, False,
             ['achi acho?', '', 'achi acho'],
             [u'ʌtʃɪ ʌtʃʊ ', '', u'ʌtʃɪ ʌtʃʊ ']),
         ('segments', True, True, False,
             ['achi acho?', '', 'achi acho'],
-            [u'ʌtʃɪ ʌtʃʊ?', '', u'ʌtʃɪ ʌtʃʊ ']),
+            [u'ʌtʃɪ ʌtʃʊ? ', '', u'ʌtʃɪ ʌtʃʊ ']),
         ('festival', False, False, False,
             ['hello world!', '', 'goodbye'],
             ['hhaxlow werld ', 'guhdbay ']),
         ('festival', False, True, False,
             ['hello world!', '', 'goodbye'],
-            ['hhaxlow werld!', 'guhdbay ']),
+            ['hhaxlow werld! ', 'guhdbay ']),
         ('festival', True, False, False,
             ['hello world!', '', 'goodbye'],
             ['hhaxlow werld ', '', 'guhdbay ']),
         ('festival', True, True, False,
             ['hello world!', '', 'goodbye'],
-            ['hhaxlow werld!', '', 'guhdbay ']),
+            ['hhaxlow werld! ', '', 'guhdbay ']),
         ('espeak', False, False, True,
             ['hello world!', '', 'goodbye'],
             [('hello world!', 'həloʊ wɜːld '), ('goodbye', 'ɡʊdbaɪ ')]),
         ('espeak', False, True, True,
             ['hello world!', '', 'goodbye'],
-            [('hello world!', 'həloʊ wɜːld!'), ('goodbye', 'ɡʊdbaɪ ')]),
+            [('hello world!', 'həloʊ wɜːld! '), ('goodbye', 'ɡʊdbaɪ ')]),
         ('espeak', True, False, True,
             ['hello world!', '', 'goodbye'],
             [('hello world!', 'həloʊ wɜːld '), ('', ''), ('goodbye', 'ɡʊdbaɪ ')]),
         ('espeak', True, True, True,
             ['hello world!', '', 'goodbye'],
-            [('hello world!', 'həloʊ wɜːld!'), ('', ''), ('goodbye', 'ɡʊdbaɪ ')]),
+            [('hello world!', 'həloʊ wɜːld! '), ('', ''), ('goodbye', 'ɡʊdbaɪ ')]),
         ('segments', False, False, True,
             ['achi acho?', '', 'achi acho'],
             [('achi acho?', 'ʌtʃɪ ʌtʃʊ '), ('achi acho', u'ʌtʃɪ ʌtʃʊ ')]),
         ('segments', False, True, True,
             ['achi acho?', '', 'achi acho'],
-            [('achi acho?', 'ʌtʃɪ ʌtʃʊ?'), ('achi acho', u'ʌtʃɪ ʌtʃʊ ')]),
+            [('achi acho?', 'ʌtʃɪ ʌtʃʊ? '), ('achi acho', u'ʌtʃɪ ʌtʃʊ ')]),
         ('segments', True, False, True,
             ['achi acho?', '', 'achi acho'],
             [('achi acho?', u'ʌtʃɪ ʌtʃʊ '), ('', ''), ('achi acho', u'ʌtʃɪ ʌtʃʊ ')]),
         ('segments', True, True, True,
             ['achi acho?', '', 'achi acho'],
-            [('achi acho?', u'ʌtʃɪ ʌtʃʊ?'), ('', ''), ('achi acho', u'ʌtʃɪ ʌtʃʊ ')]),
+            [('achi acho?', u'ʌtʃɪ ʌtʃʊ? '), ('', ''), ('achi acho', u'ʌtʃɪ ʌtʃʊ ')]),
         ('festival', False, False, True,
             ['hello world!', '', 'goodbye'],
             [('hello world!', 'hhaxlow werld '), ('goodbye', 'guhdbay ')]),
         ('festival', False, True, True,
             ['hello world!', '', 'goodbye'],
-            [('hello world!', 'hhaxlow werld!'), ('goodbye', 'guhdbay ')]),
+            [('hello world!', 'hhaxlow werld! '), ('goodbye', 'guhdbay ')]),
         ('festival', True, False, True,
             ['hello world!', '', 'goodbye'],
             [('hello world!', 'hhaxlow werld '), ('', ''), ('goodbye', 'guhdbay ')]),
         ('festival', True, True, True,
             ['hello world!', '', 'goodbye'],
-            [('hello world!', 'hhaxlow werld!'), ('', ''), ('goodbye', 'guhdbay ')])])
+            [('hello world!', 'hhaxlow werld! '), ('', ''), ('goodbye', 'guhdbay ')])])
 def test_preserve_empty_lines(backend, empty_lines, punctuation, prepend_text, text, expected):
     language = 'cree' if backend == 'segments' else 'en-us'
 
diff --git a/test/test_punctuation.py b/test/test_punctuation.py
index b2206ac..64e7cec 100644
--- a/test/test_punctuation.py
+++ b/test/test_punctuation.py
@@ -21,6 +21,7 @@
 from phonemizer.backend import EspeakBackend, FestivalBackend, SegmentsBackend
 from phonemizer.punctuation import Punctuation
 from phonemizer.phonemize import phonemize
+from phonemizer.separator import Separator, default_separator
 
 # True if we are using espeak>=1.50
 ESPEAK_150 = (EspeakBackend.version() >= (1, 50))
@@ -57,23 +58,23 @@ def test_remove(inp, out):
 def test_preserve(inp):
     punct = Punctuation()
     text, marks = punct.preserve(inp)
-    assert inp == punct.restore(text, marks)
+    assert inp == punct.restore(text, marks, sep=default_separator, strip=True)
 
 
 @pytest.mark.parametrize(
-    'text, expected', [
-        (['hi; ho,"'], ['haɪ; hoʊ,']),
-        (['hi; "ho,'], ['haɪ; hoʊ,'] if ESPEAK_143 else ['haɪ;  hoʊ,']),
-        (['"hi; ho,'], ['haɪ; hoʊ,'] if ESPEAK_143 else [' haɪ; hoʊ,'])])
-def test_preserve_2(text, expected):
+    'text, expected_restore, expected_output', [
+        (['hi; ho,"'], ['hi; ho," '], ['haɪ; hoʊ, ']),
+        (['hi; "ho,'], ['hi; "ho, '], ['haɪ; hoʊ, '] if ESPEAK_143 else ['haɪ;  hoʊ, ']),
+        (['"hi; ho,'], ['"hi; ho, '], ['haɪ; hoʊ, '] if ESPEAK_143 else [' haɪ; hoʊ, '])])
+def test_preserve_2(text, expected_restore, expected_output):
     marks = ".!;:,?"
     punct = Punctuation(marks=marks)
-    assert text == punct.restore(*punct.preserve(text))
+    assert expected_restore == punct.restore(*punct.preserve(text), sep=default_separator, strip=False)
 
     output = phonemize(
         text, backend="espeak",
         preserve_punctuation=True, punctuation_marks=marks)
-    assert output == expected
+    assert output == expected_output
 
 
 def test_custom():
@@ -93,7 +94,7 @@ def test_espeak():
     expected1 = 'həloʊ wɜːld'
     expected2 = 'həloʊ, wɜːld!'
     expected3 = 'həloʊ wɜːld '
-    expected4 = 'həloʊ, wɜːld!'
+    expected4 = 'həloʊ, wɜːld! '
 
     out1 = EspeakBackend('en-us', preserve_punctuation=False).phonemize(
         [text], strip=True)[0]
@@ -117,7 +118,7 @@ def test_festival():
     expected1 = 'hhaxlow werld'
     expected2 = 'hhaxlow, werld!'
     expected3 = 'hhaxlow werld '
-    expected4 = 'hhaxlow, werld!'
+    expected4 = 'hhaxlow, werld! '
 
     out1 = FestivalBackend('en-us', preserve_punctuation=False).phonemize(
         [text], strip=True)[0]
@@ -141,7 +142,7 @@ def test_segments():
     expected1 = 'ʌtʃɪ ʌtʃʊ'
     expected2 = 'ʌtʃɪ, ʌtʃʊ!'
     expected3 = 'ʌtʃɪ ʌtʃʊ '
-    expected4 = 'ʌtʃɪ, ʌtʃʊ!'
+    expected4 = 'ʌtʃɪ, ʌtʃʊ! '
 
     out1 = SegmentsBackend('cree', preserve_punctuation=False).phonemize(
         [text], strip=True)[0]
@@ -162,26 +163,26 @@ def test_segments():
 
 # see https://github.com/bootphon/phonemizer/issues/54
 @pytest.mark.parametrize(
-    'text', ["!'", "'!", "!'!", "'!'"])
-def test_issue_54(text):
+    'text, expected', [("!'", "! "), ("'!", "! "), ("!'!", "!! "), ("'!'", "! ")])
+def test_issue_54(text, expected):
     output = phonemize(
         [text], language='en-us', backend='espeak',
         preserve_punctuation=True)[0]
-    assert text.replace("'", '') == output
+    assert expected == output
 
 
 # see https://github.com/bootphon/phonemizer/issues/55
 @pytest.mark.parametrize(
     'backend, marks, text, expected', [
-        ('espeak', 'default', ['"Hey! "', '"hey,"'], ['"heɪ! "', '"heɪ,"']),
-        ('espeak', '.!;:,?', ['"Hey! "', '"hey,"'],
-         ['heɪ! ', 'heɪ,'] if ESPEAK_150 else [' heɪ! ', ' heɪ,']),
-        ('espeak', 'default', ['! ?', 'hey!'], ['! ?', 'heɪ!']),
-        ('espeak', '!', ['! ?', 'hey!'], ['! ', 'heɪ!']),
-        ('segments', 'default', ['! ?', 'hey!'], ['! ?', 'heːj!']),
+        ('espeak', 'default', ['"Hey! "', '"hey,"'], ['"heɪ! " ', '"heɪ," ']),
+        ('espeak', '.!;:,?', ['"Hey! " ', '"hey," '],
+         ['heɪ! ', 'heɪ, '] if ESPEAK_150 else [' heɪ! ', ' heɪ, ']),
+        ('espeak', 'default', ['! ?', 'hey!'], ['! ? ', 'heɪ! ']),
+        ('espeak', '!', ['! ?', 'hey!'], ['! ', 'heɪ! ']),
+        ('segments', 'default', ['! ?', 'hey!'], ['! ? ', 'heːj! ']),
         ('segments', '!', ['! ?', 'hey!'], ValueError),
-        ('festival', 'default', ['! ?', 'hey!'], ['! ?', 'hhey!']),
-        ('festival', '!', ['! ?', 'hey!'], ['! ', 'hhey!'])])
+        ('festival', 'default', ['! ?', 'hey!'], ['! ? ', 'hhey! ']),
+        ('festival', '!', ['! ?', 'hey!'], ['! ', 'hhey! '])])
 def test_issue55(backend, marks, text, expected):
     if marks == 'default':
         marks = Punctuation.default_marks()