-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Python 3 #3
Conversation
Many thanks. I am wondering whether it would work with the pre-trained models I released ? |
I tested it with "model256" and it does work. |
I reran the model with your "python3" branch on the UD Norwegian NynorskLIA corpus. It first returned a "UnicodeDecodeError". After fixing I got warnings "UnicodeWarning: Unicode equal comparison failed to convert both arguments to Unicode - interpreting them as being unequal", and then "IndexError: list index out of range" in the load_embeddings_file function. I do not have time to further support this, as I am doing a different project. So I'd to inform you that I close this pull request. Thanks! |
I disagree with closing it even if you don't have time, you could just leave it open. But anyway, can you share the first fix you did and what you run to test it exactly? Maybe I can fix it. |
https://drive.google.com/open?id=1d9mlsx_kmSQmwpY9I57Fmvrw6cE8rQlj The errors I got when I ran with python2. |
This worked fine for me (python 3): python jPTDP.py --predict --model model256 --params model256.params --test UD_Norwegian-NynorskLIA/no_nynorsklia-ud-test.conllu --outdir output2 --output abc python jPTDP.py --outdir output --train UD_Norwegian-NynorskLIA/no_nynorsklia-ud-train.conllu --dev UD_Norwegian-NynorskLIA/no_nynorsklia-ud-dev.conllu I don't know what the other files are for (vectors and the XML file). |
you were using python 2 or 3? |
I pushed a fix to the branch, which I think it doesn't appear here because it's closed. The issue was that I tested before with English files, that have no accent. Now it's working both for python 2 and 3, converting training and testing. Please, test it yourself and let me know if you run into troubles. |
Thanks. I reopen the pull request. I do not have time to test it now. But I will spend sometime to do it! |
Note that you need to install the package |
I retested with python2 (I could not test with python3 as numpy was broken with python3 on my Mac). Anyway, the major issue is that running the new "porting" code with the pre-trained models I released significantly degraded the original performance. You can verify it on the the UD Norwegian NynorskLIA corpus yourself. In other words, with the new code it would require to retrain all models, and I do not have time to redo it. So I have decided to close this pull request. Instead, I might make a link to your porting code. Thanks. |
I tested converting, training and predicting in both Python 2 and 3.7 and it worked fine.