ibayer / fastFM

fastFM: A Library for Factorization Machines
http://ibayer.github.io/fastFM
Other
1.08k stars 204 forks source link

Recompile for python 3.7 #151

Open zeeraktalat opened 5 years ago

zeeraktalat commented 5 years ago

FastFM fails to install on python 3.7 due to updates in Cython. I'd put in a pull request, but due to the split between FastFM-core and fastFM I'm not sure where to submit the PR.

Steps to reproduce issue:

  1. Fresh python > 3.7.1 environment
  2. pip install fastFM

Steps to fix:

  1. clone the library
  2. Install latest Cython (0.29.1)
  3. Recompile fastFM
ibayer commented 5 years ago

@ZeerakW Thanks for raising the issue.

Has fastFM been working with 3.7 before or is a more recent cython need to compile fastFM for 3.7?

zeeraktalat commented 5 years ago

I'm not sure if it's been working for 3.7 before, just came across your library a couple of days ago. But it seems that specifically Cython 0.29 has been used so a more recent version of cython is needed.

I managed to compile it using Cython 0.29 but I wasn't sure where to submit a PR. I think in fastFM-core, but not sure - if it is in core then let me know and I can submit a PR later today.

ibayer commented 5 years ago

@ZeerakW

if it is in core then let me know and I can submit a PR later today.

It's not, core contains only the C code. I'm just still a bit puzzled what you changes are but a PR will make this clear. :-)

zeeraktalat commented 5 years ago

The only changes that are happening are in core so I'm not sure what the difference is :/.


On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
    modified:   fastFM-core (untracked content)

no changes added to commit```
ibayer commented 5 years ago

@ZeerakW Ah, okay I think I understand the situation now. You only changed the Cython version to 0.29 to make source install work. Since we don't include the Cython generated files into the repo nothing else needs to be changed.

The only changes that are happening are in core

This are just the created binary files that are not included into the repo.

So the conclusion seems to be that: for python 3.7 the Cython requirement needs to be raised to 0.29

zeeraktalat commented 5 years ago

So the conclusion seems to be that: for python 3.7 the Cython requirement needs to be raised to 0.29

Yup, exactly. :)

RaitoBezarius commented 4 years ago

It'd be very cool if a wheel was pushed for Python 3.7 on PyPI