kylebgorman / pynini

Read-only mirror of Pynini
http://pynini.opengrm.org
Apache License 2.0
120 stars 26 forks source link

About incompatibility of pynini 1.9.1 (openfst 1.6.7) with Cython #3

Closed underdogliu closed 6 years ago

underdogliu commented 6 years ago

OS: Ubuntu 16.04 Running dependency: Python 3.6.5 + OpenFST 1.6.7 + re2 + Pynini 1.9.1

Dear Kyle,

I'm currently playing with the newest ver. of pynini. However, after compiling openfst and re2, when executing following command in src/:

cython -3 --cplus -o pywrapfst.cc pywrapfst.pyx

I got following errors relevant to cython:

Error compiling Cython file:
------------------------------------------------------------
...

    Constructs a non-member weight in the semiring.
    """
    return _NoWeight(weight_type)

  def __eq__(Weight w1, Weight w2):
 ^
------------------------------------------------------------

pywrapfst.pyx:432:2: Special method __eq__ must be implemented via __richcmp__

Error compiling Cython file:
------------------------------------------------------------
...
    return _NoWeight(weight_type)

  def __eq__(Weight w1, Weight w2):
    return fst.Eq(deref(w1._weight), deref(w2._weight))

  def __ne__(Weight w1, Weight w2):
 ^
------------------------------------------------------------

pywrapfst.pyx:435:2: Special method __ne__ must be implemented via __richcmp__

Error compiling Cython file:
------------------------------------------------------------
...
      FstIOError: Write to string failed.

    See also: `read_from_string`.
    """
    cdef stringstream sstrm
    if not self._fst.get().Write(sstrm, "write_to_string"):
                                       ^
------------------------------------------------------------

pywrapfst.pyx:1769:40: Unicode literals do not support coercion to C types other than Py_UNICODE/Py_UCS4 (for characters) or Py_UNICODE* (for strings).

So seems like pynini1.9.1 is not fully compatible with Python3 (while pynini 1.8 is as I tried) due to unicode literal errors. Is that true or I did something wrong? If so, do we have a plan in short term on supporting pynini 1.9.1 with python 3?

I'm having trouble using TWiki so I post my question here. If it's unfortunately inappropriate please say and I'll get back to TWiki again. Thanks for being helpful!

kylebgorman commented 6 years ago

Is it possible you're using a older version of Cython (try: cython --version)?

For instance, this issue:

pywrapfst.pyx:432:2: Special method eq must be implemented via richcmp

was fixed in Cython 0.26 or 0.27, IIRC.

I do see the second error, though on Cython 0.28.1.

In this case, it's pretty easy to fix: add a 'b' so Cython knows it's a byte literal. Thus the line will read:

if not self._fst.get().Write(sstrm, b"write_to_string"):

This is the easiest sort of compatibility issue because I believe that fix will be portable back to Python 2.

As for your general question, I would love to add Python 3 support so long as it doesn't break Python 2 support. That's one reason I created the repo here---to encourage compatibility fixes like the above. Understand though that I wrote this in a professional capacity and my employer is a Python 2 shop for the time being.

underdogliu commented 6 years ago

@kylebgorman Sincerely thanks for your detailed reply! Yes I was using Cython 0.25.1. So it is an old version for pynini 1.9.1 right? (It worked for pynini 1.8 with openfst 1.6.5, other configurations were same)

kylebgorman commented 6 years ago

According to the README I used Cython 0.27.3 to generate the .cc files for Pynini 1.9.1. I usually just update my Cython ahead of each release.

On Thu, Apr 5, 2018 at 7:01 AM, Searcher notifications@github.com wrote:

yes I'm using Cython 0.25.1. So it is an old version for pynini 1.9.1 right? (It worked for pynini 1.8 with openfst 1.6.5, other configurations were same)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kylebgorman/Pynini/issues/3#issuecomment-378898577, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJuOUhI4r5Lc9lgHwk3wig48bp2hzVKks5tlflygaJpZM4TG4yj .

underdogliu commented 6 years ago

Got it. I'll make a note for this issue. Thanks!

kylebgorman commented 6 years ago

Pynini 2.0 has built-in Python 3 support, please check out at pynini.opengrm.org

On Fri, Apr 13, 2018 at 5:17 AM Xuechen Liu notifications@github.com wrote:

Closed #3 https://github.com/kylebgorman/Pynini/issues/3.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kylebgorman/Pynini/issues/3#event-1572811146, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJuOfsMqvsQzOlYhfF_iHDmS2wyrQJiks5toG0TgaJpZM4TG4yj .