Repository: bootphon/phonemizer (language: python)

Issue:
Request: more flexibility around punctuation definitions
**Is your feature request related to a problem? Please describe.**
I'd like more flexibility in defining punctuation, ideally by having access directly to the regex. 

Specifically, instead of defining the characters to be counted as punctuation, I think it'd be more useful to me to define which characters are words to be phonemized, and treat everything else as punctuation. 

**Describe the solution you'd like**
Something as broad as `[^\p{L}\p{M}0-9']` could work as a default, which from what I understand would capture everything that's not a number, unicode letter or its diacritics. 

That may be overly broad, though, because I've run into trouble with espeak and characters from Cyrillic and Korean sets already, and I'd imagine characters from other less-supported languages could also be problematic. 

**Describe alternatives you've considered**
In my local copy of phonemizer, I've played with hard-coding the punctuation regex like so:
```
marks = "[^a-zA-ZÀ-ÖØ-öø-ÿ0-9',.$@&+\\-=/\\\]"
self._marks_re = re.compile(fr'(\s*{marks}+\s*|\s*(?<!\d),\s*|\s*(?<!\d)\.(?!\d)\s*)+')
```
which captures everything that's not a latin character or a set of marks that the backends can pronounce, like "æt" for "@". 

The back half of this is also an attempt to handle the problem raised in #87, though I haven't tested it much, and there may be some cases where it breaks.



Discussion / hints:
@hadware Thinking about this request again. How about adding an option to define a punctuation regex directly? It would override the default punctuation marks, and probably throw an exception if a user tries to define both custom punctuation and a punctuation regex? This is probably the easiest path to get the features I need, without changing the default behavior people have come to expect.

I can try working something up if that feature makes sense to you.

Hi @jncasey and thank you for your implication in this project!

Actually, you can already provide punctuation marks as parameter, both from [CLI](https://github.com/bootphon/phonemizer/blob/master/phonemizer/main.py#L298) and [API](https://github.com/bootphon/phonemizer/blob/master/phonemizer/phonemize.py#L51) as a `str` that is fed to the `Punctuation` class and compiled as a regex.

If you prefer to specify directly a precompiled regex instead of a string, it would be possible, for instance, by adding a `isinstance` check in `Punctuation.__init__` ([here](https://github.com/bootphon/phonemizer/blob/master/phonemizer/punctuation.py#L63)). But, in that case, you'll lose the `Punctuation.mark` attribute. I don't think it is really used in the code, so we can imagine raising an exception when accessing a `mark` attribute from a `Punctuation` instance built from a regex...
I think I agree with @mmmaat 's suggestion. I'd rather have the signature of `Punctuation.__init__` be something like

```python
def __init__(self, marks: Optional[str] = None, pattern: Optional[Union[str, re.Pattern]] = None):
   ...
```

to make things more obvious maybe? (and if both args are `None`, it'd still default to `DEFAULT_MARKS`)
Thanks, @mmmaat!

I'm aware that it's possible to pass punctuation marks as a string, but in my case, I'm looking to treat all characters that aren't alphanumeric (or accented alpha) as punctuation. That would include the standard marks on the keyboard, but also emoji and lots of other unicode characters. So it's not really practical to pass a list of every possibility. 

Which is why I'd like to pass the punctuation regex directly as a parameter. 

I like your thought of an additional `isinstance` check allowing for marks to be a precompiled regex. That would work perfectly in my use case, and require minimal changes everywhere else. But with that approach, it wouldn't be possible to expose the functionality to CLI version, right? There'd be no way to know if the `--punctuation-marks` parameter was intended as a regex or not.

Does that matter?
@hadware Oops, we responded at the same time. 

I can work off of that signature. And the corresponding new `phonemize()` param would be `punctuation_pattern` and the CLI param would be `--punctuation-pattern`?
Yes! That would be perfect!
(And, again, you can start right away with a [WIP] PR if you feel like working on this now)

PS: you're the best :rocket: 
Ok @jncasey my idea is incompatible with the CLI.

Why not to add  a boolean flag `--punctuation-marks-is-regex` defaulting to `False` from the CLI? The name is pretty long, OK, but the `punctuation_pattern` name is very ambiguous to me and it's confusing to have both marks and pattern for the same functionality.

What do you think?
In that case, the flag would be checked in main.py, and if it's true, simply `args.punctuation_marks = re.compile(args.punctuation_marks)`? That's pretty clean.

But ultimately, it seems like a stylistic choice. Happy to implement it either way – I'll follow the lead of whatever you guys collectively decide. 

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c5bfd7a..453eac5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,17 +3,48 @@
 Version numbers follow [semantic versioning](https://semver.org)
 
 
+## not yet released
+
+* **bug fixes**
+
+  * Fixed a bug when trying to restore punctuation on very long text.
+    See [issue #108](https://github.com/bootphon/phonemizer/issues/108)
+
+* **improvements**
+
+  * Improved consistency with the handling of word separators when
+    preserving punctuation, and when using a word separator that is
+    not a literal space character. See [issue
+    #106](https://github.com/bootphon/phonemizer/issues/106)
+
+* **new features**
+
+  * Added the option to define punctuation with a regular expression.
+    Previously only strings were accepted. See [PR
+    #120](https://github.com/bootphon/phonemizer/pull/120)
+
+    * In the python API, the `punctuation_marks` parameter can now be
+      passed to `phonemize` (or a backend constructor) as a `re.Pattern`
+      that defines which characters will be matched as punctuation.
+      Passing `punctuation_marks` as a str will continue to function as
+      before, treating each character in the string as a punctuation mark.
+
+    * Added the optional parameter `--punctuation_marks_is_regex` to the CLI
+      interface. When used, the CLI will attempt to compile a `re.Pattern`
+      from the value passed to `--punctuation-marks`.
+
+
 ## phonemizer-3.1.1
 
 * **improvements**
 
-  * Preserve empty lines in texts when using  `--preserve-empty-lines`. 
-    Without this option, empty lines used to be automatically dropped. 
+  * Preserve empty lines in texts when using  `--preserve-empty-lines`.
+    Without this option, empty lines used to be automatically dropped.
     See [PR #103](https://github.com/bootphon/phonemizer/pull/103)
 
 * **new features**
 
-  * Type hinted most of `phonemizer`'s API. This makes the usage of our API 
+  * Type hinted most of `phonemizer`'s API. This makes the usage of our API
     a bit clearer, and can be easily leveraged by IDE's and type checkers to
     prevent typing issues.
 
diff --git a/phonemizer/backend/base.py b/phonemizer/backend/base.py
index ce132c3..1667809 100644
--- a/phonemizer/backend/base.py
+++ b/phonemizer/backend/base.py
@@ -16,8 +16,9 @@
 
 import abc
 import itertools
+import re
 from logging import Logger
-from typing import Optional, List, Any, Dict, Tuple, Union
+from typing import Optional, List, Any, Dict, Tuple, Union, Pattern
 
 import joblib
 
@@ -43,9 +44,9 @@ class BaseBackend(abc.ABC):
       phonemized output. Not supported by the 'espeak-mbrola' backend. Default
       to False and remove all the punctuation.
 
-    punctuation_marks (str): The punctuation marks to consider when dealing
-      with punctuation, either for removal or preservation. Default to
-      Punctuation.default_marks().
+    punctuation_marks (str or re.Pattern): The punctuation marks to consider when
+      dealing with punctuation, either for removal or preservation. Can be defined
+      as a string or regular expression. Default to Punctuation.default_marks().
 
     logger (logging.Logger): the logging instance where to send
       messages. If not specified, use the default system logger.
@@ -58,7 +59,7 @@ class BaseBackend(abc.ABC):
     """
 
     def __init__(self, language: str,
-                 punctuation_marks: Optional[str] = None,
+                 punctuation_marks: Optional[Union[str, Pattern]] = None,
                  preserve_punctuation: bool = False,
                  logger: Optional[Logger] = None):
 
diff --git a/phonemizer/backend/espeak/base.py b/phonemizer/backend/espeak/base.py
index fe6ec6e..387598b 100644
--- a/phonemizer/backend/espeak/base.py
+++ b/phonemizer/backend/espeak/base.py
@@ -16,7 +16,7 @@
 
 import abc
 from logging import Logger
-from typing import Optional
+from typing import Optional, Union, Pattern
 
 from phonemizer.backend.base import BaseBackend
 from phonemizer.backend.espeak.wrapper import EspeakWrapper
@@ -33,7 +33,7 @@ class BaseEspeakBackend(BaseBackend):
 
     """
     def __init__(self, language: str,
-                 punctuation_marks: Optional[str] = None,
+                 punctuation_marks: Optional[Union[str, Pattern]] = None,
                  preserve_punctuation: bool = False,
                  logger: Optional[Logger] = None):
         super().__init__(
diff --git a/phonemizer/backend/espeak/espeak.py b/phonemizer/backend/espeak/espeak.py
index 1ebf4b6..34e3faf 100644
--- a/phonemizer/backend/espeak/espeak.py
+++ b/phonemizer/backend/espeak/espeak.py
@@ -17,7 +17,7 @@
 import itertools
 import re
 from logging import Logger
-from typing import Optional, Tuple, List, Union
+from typing import Optional, Tuple, List, Union, Pattern
 
 from phonemizer.backend.espeak.base import BaseEspeakBackend
 from phonemizer.backend.espeak.language_switch import (
@@ -35,7 +35,7 @@ class EspeakBackend(BaseEspeakBackend):
 
     # pylint: disable=too-many-arguments
     def __init__(self, language: str,
-                 punctuation_marks: Optional[str] = None,
+                 punctuation_marks: Optional[Union[str, Pattern]] = None,
                  preserve_punctuation: bool = False,
                  with_stress: bool = False,
                  tie: Union[bool, str] = False,
diff --git a/phonemizer/backend/festival/festival.py b/phonemizer/backend/festival/festival.py
index dfb8962..868c92b 100644
--- a/phonemizer/backend/festival/festival.py
+++ b/phonemizer/backend/festival/festival.py
@@ -24,7 +24,7 @@
 import tempfile
 from logging import Logger
 from pathlib import Path
-from typing import Optional, Dict, List, IO
+from typing import Optional, Dict, List, IO, Union, Pattern
 
 from phonemizer.backend.base import BaseBackend
 from phonemizer.backend.festival import lispy
@@ -40,7 +40,7 @@ class FestivalBackend(BaseBackend):
     _FESTIVAL_EXECUTABLE = None
 
     def __init__(self, language: str,
-                 punctuation_marks: Optional[str] = None,
+                 punctuation_marks: Optional[Union[str, Pattern]] = None,
                  preserve_punctuation: bool = False,
                  logger: Optional[Logger] = None):
         super().__init__(
diff --git a/phonemizer/backend/segments.py b/phonemizer/backend/segments.py
index c455349..201450c 100644
--- a/phonemizer/backend/segments.py
+++ b/phonemizer/backend/segments.py
@@ -16,7 +16,7 @@
 
 import pathlib
 from logging import Logger
-from typing import Optional, Dict, List
+from typing import Optional, Dict, List, Union, Pattern
 
 import segments
 
@@ -34,7 +34,7 @@ class SegmentsBackend(BaseBackend):
     """
 
     def __init__(self, language: str,
-                 punctuation_marks: Optional[str] = None,
+                 punctuation_marks: Optional[Union[str, Pattern]] = None,
                  preserve_punctuation: bool = False,
                  logger: Optional[Logger] = None):
         # will be initialized in _init_language() from super().__init__()
diff --git a/phonemizer/main.py b/phonemizer/main.py
index 0ef4801..9b85c1d 100644
--- a/phonemizer/main.py
+++ b/phonemizer/main.py
@@ -18,6 +18,7 @@
 import argparse
 import os
 import sys
+import re
 
 import pkg_resources
 
@@ -301,6 +302,11 @@ def parse_args():
         default=punctuation.Punctuation.default_marks(),
         help='''the marks to consider during punctuation processing (either
         for removal or preservation). Default is %(default)s.''')
+    group.add_argument(
+        '--punctuation-marks-is-regex',
+        action='store_true',
+        help="""interpret the '--punctuation-marks' parameter as a regex.
+        Default is to interpret as a string.""")
 
     return parser.parse_args()
 
@@ -388,6 +394,16 @@ def main():
     else:
         input_output_separator = False
 
+    if args.punctuation_marks_is_regex:
+        try:
+            log.debug('punctuation marks is regex %s', args.punctuation_marks)
+            args.punctuation_marks = re.compile(args.punctuation_marks)
+        except re.error:
+            # manually close the open streams for windows
+            streamin.close()
+            streamout.close()
+            raise ValueError(f"can't compile regex pattern from {args.punctuation_marks}")
+
     # phonemize the input text
     out = phonemize(
         streamin.readlines(),
diff --git a/phonemizer/phonemize.py b/phonemizer/phonemize.py
index 5a5a77f..a77da80 100644
--- a/phonemizer/phonemize.py
+++ b/phonemizer/phonemize.py
@@ -23,7 +23,7 @@
 import os
 import sys
 from logging import Logger
-from typing import Optional, Union, List
+from typing import Optional, Union, List, Pattern
 
 from typing_extensions import Literal
 
@@ -48,7 +48,7 @@ def phonemize(  # pylint: disable=too-many-arguments
         prepend_text: bool = False,
         preserve_empty_lines: bool = False,
         preserve_punctuation: bool = False,
-        punctuation_marks: str = Punctuation.default_marks(),
+        punctuation_marks: Union[str, Pattern] = Punctuation.default_marks(),
         with_stress: bool = False,
         tie: Union[bool, str] = False,
         language_switch: LanguageSwitch = 'keep-flags',
@@ -114,9 +114,10 @@ def phonemize(  # pylint: disable=too-many-arguments
       in the phonemized output. Not supported by the 'espeak-mbrola' backend.
       Default to False and remove all the punctuation.
 
-    punctuation_marks (str, optional): The punctuation marks to consider when
-      dealing with punctuation, either for removal or preservation. Default to
-      Punctuation.default_marks().
+    punctuation_marks (str or re.Pattern, optional): The punctuation marks to
+      consider when dealing with punctuation, either for removal or
+      preservation. Can be defined as a string or regular expression. Default
+      to Punctuation.default_marks().
 
     with_stress (bool, optional): This option is only valid for the 'espeak'
       backend. When True the stresses on phonemes are present (stresses
diff --git a/phonemizer/punctuation.py b/phonemizer/punctuation.py
index 5c1dddc..cfda53d 100644
--- a/phonemizer/punctuation.py
+++ b/phonemizer/punctuation.py
@@ -16,7 +16,7 @@
 
 import collections
 import re
-from typing import List, Union, Tuple
+from typing import List, Union, Tuple, Pattern
 
 from phonemizer.utils import str2list
 from phonemizer.separator import Separator
@@ -38,15 +38,15 @@ class Punctuation:
 
     Parameters
     ----------
-    marks (str) : The list of punctuation marks to considerate for processing
-        (either removal or preservation). Each mark must be made of a single
-        character. Default to Punctuation.default_marks().
+    marks (str or re.Pattern) : The punctuation marks to consider for processing
+        (either removal or preservation). If a string, each mark must be made of
+        a single character. Default to Punctuation.default_marks().
 
     """
 
-    def __init__(self, marks: str = _DEFAULT_MARKS):
+    def __init__(self, marks: Union[str, Pattern] = _DEFAULT_MARKS):
         self._marks: str = None  # noqa
-        self._marks_re: re.Pattern[str] = None  # noqa
+        self._marks_re: Pattern[str] = None  # noqa
         self.marks = marks
 
     @staticmethod
@@ -57,17 +57,24 @@ def default_marks():
     @property
     def marks(self):
         """The punctuation marks as a string"""
-        return self._marks
+        if self._marks:
+            return self._marks
+        raise ValueError('punctuation initialized from regex, cannot access marks as a string')
 
     @marks.setter
-    def marks(self, value: str):
-        if not isinstance(value, str):
-            raise ValueError('punctuation marks must be defined as a string')
-        self._marks = ''.join(set(value))
-
-        # catching all the marks in one regular expression: zero or more spaces
-        # + one or more marks + zero or more spaces.
-        self._marks_re = re.compile(fr'(\s*[{re.escape(self._marks)}]+\s*)+')
+    def marks(self, value: Union[str, Pattern]):
+        if isinstance(value, Pattern):
+            # catch the pattern surrounded by zero or more spaces on either side
+            self._marks_re = re.compile(r'((' + value.pattern + r')|\s)+')
+            self._marks = None
+        else:
+            if not isinstance(value, str):
+                raise ValueError('punctuation marks must be defined as a string or re.Pattern')
+            self._marks = ''.join(set(value))
+
+            # catching all the marks in one regular expression: zero or more spaces
+            # + one or more marks + zero or more spaces.
+            self._marks_re = re.compile(fr'(\s*[{re.escape(self._marks)}]+\s*)+')
 
     def remove(self, text: Union[str, List[str]]) -> Union[str, List[str]]:
         """Returns the `text` with all punctuation marks replaced by spaces


Test patch:
diff --git a/test/test_main.py b/test/test_main.py
index b8c53e9..b9256a0 100644
--- a/test/test_main.py
+++ b/test/test_main.py
@@ -24,7 +24,7 @@
 
 import pytest
 
-from phonemizer.backend import EspeakMbrolaBackend
+from phonemizer.backend import EspeakMbrolaBackend, EspeakBackend
 from phonemizer import main, backend, logger
 
 
@@ -132,3 +132,27 @@ def test_festival_path():
 
     _test('hello world', 'hhaxlow werld ',
           f'--festival-executable={festival} -b festival')
+
+
+@pytest.mark.parametrize(
+    'args, expected', [
+        ('',
+         'həloʊ wɜːld θɹiː ziəɹoʊziəɹoʊ ziəɹoʊ ɔːɹ tuː fɪfti həloʊ '),
+        ('--preserve-punctuation',
+         'həloʊ, ,wɜːld? θɹiː,ziəɹoʊziəɹoʊ ziəɹoʊ, ɔːɹ tuː.fɪfti. ¿həloʊ? '),
+        ('--preserve-punctuation '
+         '--punctuation-marks-is-regex '
+         '--punctuation-marks "[^a-zA-ZÀ-ÖØ-öø-ÿ0-9\'\\-]"',
+         'həloʊ, ,wɜːld? ‡ θɹiː,ziəɹoʊziəɹoʊ ziəɹoʊ, ɔːɹ tuː.fɪfti. ¿həloʊ? '),
+        ('--preserve-punctuation '
+         '--punctuation-marks-is-regex '
+         '--punctuation-marks "[;:\\!?¡¿—…\\\"«»“”]|[,.](?!\\d)"',
+         'həloʊ, ,wɜːld? θɹiː θaʊzənd, ɔːɹ tuː pɔɪnt faɪv ziəɹoʊ. ¿həloʊ? ')])
+def test_punctuation_is_regex(args, expected):
+    print(args)
+    _test("hello, ,world? ‡ 3,000, or 2.50. ¿hello?", expected, args)
+
+
+def test_invalid_punctuation_regex():
+    with pytest.raises(SystemExit):
+        _test('hello world', None, '--punctuation-marks-is-regex --punctuation-marks "[*,"')
diff --git a/test/test_punctuation.py b/test/test_punctuation.py
index 64e7cec..1948c08 100644
--- a/test/test_punctuation.py
+++ b/test/test_punctuation.py
@@ -17,6 +17,7 @@
 # pylint: disable=missing-docstring
 
 import pytest
+import re
 
 from phonemizer.backend import EspeakBackend, FestivalBackend, SegmentsBackend
 from phonemizer.punctuation import Punctuation
@@ -204,3 +205,27 @@ def test_issue55(backend, marks, text, expected):
                 # It ends with a segmentation fault. This seems to only appear
                 # with festival-2.5 (but is working on travis and docker image)
                 pass
+
+
+@pytest.mark.parametrize(
+    'punctuation_marks, text, expected', [
+        (';:,.!?¡—…"«»“”',
+         'hello, ,world? ‡ 3,000, or 2.50. ¿hello?',
+         'həloʊ, ,wɜːld? θɹiː,ziəɹoʊziəɹoʊ ziəɹoʊ, ɔːɹ tuː.fɪfti. həloʊ? '),
+        (re.compile(r"[^a-zA-ZÀ-ÖØ-öø-ÿ0-9'$@&+%\-=/\\]"),
+         'hello, ,world? ‡ 3,000, or 2.50. ¿hello?',
+         'həloʊ, ,wɜːld? ‡ θɹiː,ziəɹoʊziəɹoʊ ziəɹoʊ, ɔːɹ tuː.fɪfti. ¿həloʊ? '),
+        (re.compile(r"[^a-zA-ZÀ-ÖØ-öø-ÿ0-9',.$@&+%\-=/\\]|[,.](?!\d)"),
+         'hello, ,world? ‡ 3,000, or 2.50. ¿hello?',
+         'həloʊ, ,wɜːld? ‡ θɹiː θaʊzənd, ɔːɹ tuː pɔɪnt faɪv ziəɹoʊ. ¿həloʊ? ')
+])
+def test_punctuation_marks_regex(punctuation_marks, text, expected):
+    assert expected == phonemize(
+        text, preserve_punctuation=True, punctuation_marks=punctuation_marks)
+
+
+def test_marks_getter_with_regex():
+    marks_re = re.compile(r"[^a-zA-Z0-9]")
+    punct = Punctuation(marks_re)
+    with pytest.raises(ValueError):
+        punct.marks == marks_re