Skip to content
This repository has been archived by the owner on Dec 6, 2023. It is now read-only.

Recompile with new Cython #154

Closed
wants to merge 1 commit into from
Closed

Recompile with new Cython #154

wants to merge 1 commit into from

Conversation

StrikerRUS
Copy link
Contributor

Fixed #153.

Linking previous recompilation: #134.

Root cause:

The tp_print slot of PyTypeObject has been removed. It was used for printing objects to files in Python 2.7 and before. Since Python 3.0, it has been ignored and unused. (Contributed by Jeroen Demeyer in bpo-36974.)
https://docs.python.org/dev/whatsnew/3.9.html#id3

https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_vectorcall_offset

Proof that it works:
https://github.com/BayesWitnesses/m2cgen/blob/979fbd8b1ae048f2a843f59e2c2755c75dd816ac/requirements-test.txt#L6
https://github.com/BayesWitnesses/m2cgen/pull/371/checks?check_run_id=2428248886

Gently ping @mblondel @MechCoder @zermelozf @fabianp @vene according to

The project is indeed no longer actively maintained but hot fixes like this to follow the latest scikit-learn are very welcome.
#142 (comment)

Also, I believe this hotfix deserves a new PyPI release to make this package compatible with Python 3.9.

Thanks for your time.

@mblondel
Copy link
Member

mblondel commented May 3, 2021

Thanks @StrikerRUS! Is random_fast.cpp the only file that needed to be recompiled?

@StrikerRUS
Copy link
Contributor Author

@mblondel Thanks for your response!

Yes. randomkit.h and randomkit.c are not produced via Cython but they are ordinary manually written files. And there are no any other C or CPP files in the repo.

@StrikerRUS
Copy link
Contributor Author

TBH, I'm very familiar with Cython, but why random_fast.cpp is not produced on the fly like all other CPP files in impl folder from corresponding *_fast.pyx files?

@mblondel
Copy link
Member

mblondel commented May 3, 2021

Good point. The cpp files used to be included and they got removed in f54f041 by @lesteve. I think random_fast.cpp should be removed too, it's probably an omission.

@StrikerRUS StrikerRUS mentioned this pull request May 3, 2021
@mblondel mblondel closed this May 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build pb on python 3.9
2 participants