daanzu / kaldi-active-grammar

Python Kaldi speech recognition with grammars that can be set active/inactive dynamically at decode-time
GNU Affero General Public License v3.0
334 stars 49 forks source link

Python 3 compatibility #1

Closed roccah closed 4 years ago

roccah commented 5 years ago

Hey, I've been checking out the package and it looks really cool!

Currently KAG is targeting Python 2 and raises exceptions in Python 3. In my brief attempts at running KAG on py3, the main changes seemed to be bytes/unicode issues and module imports. I realize it can be a bit of a pain to write polyglot. Would you be interested in supporting both versions?

daanzu commented 5 years ago

Thanks for testing! I've been trying to write for compatibility, but only been testing on 2.7, in an effort to get it released. I'd definitely like to support 3+, and intend to eventually when find time. PRs welcome!

multimeric commented 4 years ago

Do you need any help with the Python 3 porting? I notice there are some python 2 and python 3 translations in the repo at the moment

daanzu commented 4 years ago

Any tips are are appreciated. I think it will mostly just involve str/unicode issues, but it might get pretty hairy with the Kaldi interface. Not sure yet.

multimeric commented 4 years ago

Well, it's normally safe to just run 2to3 on it, and then simplify some of the messy things it does

roccah commented 4 years ago

futurize (https://python-future.org) has a bit fancier tooling for writing 2/3 polyglot code. When I tried this a few months ago it worked pretty well with a few manual interventions.

multimeric commented 4 years ago

True, but at this point I would advise against supporting Python 2 at all, which is what futurize does. The reason being, no-one should be using Python 2 at this point, and it adds a whole lot of extra junk into your code to support polyglot code.

daanzu commented 4 years ago

Actually, it appears to have been far easier than I was expecting, at least to have a working version (both python2&3). There may still be issues with actually using unicode characters, but that was true even with python2, and bears further testing. I hope to release the next version in the next few days with python3 support!

Regarding supporting python2 still, I think a lot of the dragonfly community is still running python2, so I hate to leave them in the lurch. I don't think it will be hindrance so far.

multimeric commented 4 years ago

Ah, well in that case I'll just leave this PR as an example as to how I would have done it: #2.

daanzu commented 4 years ago

This should be fixed in the new version. 🎉

@TMiguelT Thanks for the PR; it was interesting to consult.