scrapinghub / python-crfsuite

A python binding for crfsuite
MIT License
770 stars 222 forks source link

Version 0.9.5 incompatible with Python 3.7.0 #88

Closed Khris777 closed 6 years ago

Khris777 commented 6 years ago

When trying to upgrade to Python 3.7 with Conda in a Python 3 environment I'm getting

UnsatisfiableError: The following specifications were found to be in conflict:
    python-crfsuite==0.9.5 -> python[version='>=2.7,<2.8.0a0']
    python==3.7.0
lucywang000 commented 6 years ago

Since you use conda, I think it is an issue for https://github.com/conda-forge/python-crfsuite-feedstock

Of course we should also upload an wheel to pypi for py37

danmacnaughtan commented 6 years ago

It looks like pycrfsuite/_pycrfsuite.cpp needs to be rebuilt with a newer version of cython to support python 3.7.0

fuhrysteve commented 6 years ago

Resolved as of https://github.com/scrapinghub/python-crfsuite/commit/694482b1e808f7c0141ce5c3dad31704b3feffec

Tpt commented 6 years ago

Do you plan to release a new version soon with Python 3.7 support?

gwerbin commented 6 years ago

@fuhrysteve I checked out 694482b and still couldn't build against Python 3.7:

    pycrfsuite/_pycrfsuite.cpp: In function 'void __Pyx__ExceptionSave(PyThreadState*, PyObject**, PyObject**, PyObject**)':
    pycrfsuite/_pycrfsuite.cpp:14140:21: error: 'PyThreadState {aka struct _ts}' has no member named 'exc_type'; did you mean 'curexc_type'?

I don't see any changes to the C++ files in that commit. What Cython command should I run in order to rebuild them?

Edit: I fixed the issue by running ./update_cpp.sh under Python 3.7 with Cython 0.28.3.

fuhrysteve commented 6 years ago

is that update_cpp.sh something that will need to be a part of the build for this library or something? I don't know much about Cython in general. In fact, I don't know much about this library either - except that it was on the list of sub-dependencies that we need in order to go live with 3.7 haha

gwerbin commented 6 years ago

@fuhrysteve it just runs cython to generate the C++ files

fuhrysteve commented 6 years ago

Hmm, I'm still a little confused - do we need to make that a part of the build for 3.7? Or was it just an environment issue you had to sort out on your end?

gwerbin commented 6 years ago

@fuhrysteve evidently something (be it Cython or CPython) changed in 3.7, so the generated C code needed to change accordingly. This was an issue in other Cython-based projects as well, e.g. https://github.com/pygame/pygame/issues/382 . It might have been an undocumented change to the CPython C API, since I don't see anything about PyThreadState in the 3.7 release notes.

It's actually possible to pass off the Cython build process to package-installation time using a setuptools Extension, but that makes Cython an install-time dependency for the user. See https://stackoverflow.com/a/32537661/2954547

danmacnaughtan commented 6 years ago

I've been running my fork locally with the updated pycrfsuite/_pycrfsuite.cpp, which seems to run fine. I just issued a PR #91

kmike commented 6 years ago

Python 3.7 should be supported in recent python-crfsuite 0.9.6 release. Please reopen if that's not the case and you're still getting errors!