ibayer / fastFM

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

fastFM builds with only Python 3 available #105

Closed altermarkive closed 7 years ago

altermarkive commented 7 years ago

With this simple addition one can also build the module on a system with only Python 3 installed.

altermarkive commented 7 years ago

Thank you for all the work put into fastFM!

chezou commented 7 years ago

IMHO, we can use venv, virtualenv, or alternatives for this purpose. This is not the scope of a specific package.

altermarkive commented 7 years ago

There are fixes in most recent code (e.g. ibayer/fastFM@07c1676c6cd986c45522e29b34378150f3bd3152) which aren't included in the latest release (0.2.9). Therefore, I needed to build the code myself and since I'm using only Python 3 the code was not building without the fix I'm proposing - there might be others who are facing this so I decided to share :)

ibayer commented 7 years ago

Thanks for sharing this! I'm also very happy to hear that you like fastFM! Clearly your PR is one possible way to build with python3, however we choose to got with venv, virtualenv as @chezou mentioned (he has set this up and I think that's a very clean way to do it). You can look at https://github.com/ibayer/fastFM/blob/master/.travis.yml#L37 to see how you can use our approach to build from master.

altermarkive commented 7 years ago

Thank you for pointing me to your solution! Since I deploy my code in a Docker container using also the virtualenv is a bit of an overkill. I'll just wait for the next release and close the pull request.

altermarkive commented 7 years ago

Out of curiosity - why not move the viertualenv solution to the Makefile?

aaossa commented 7 years ago

I was trying to install fastFM from source too and then I realized the same as @altermarkive . I had to manually execute python3 setup.py build_ext --inplace. I think venv, virtualenv usage shoul be optional, so I would suggest using:

PYTHON ?= python

all:
    ( cd fastFM-core ; $(MAKE) lib )
    $(PYTHON) setup.py build_ext --inplace
# ...

By using this, if the user writes make then python is used, but I want to use python3 then I can execute it by typing PYTHON=python3 make. (Relevant SO question) What do you think @chezou @ibayer ?


EDIT/PS: I also would like to know if you want a PR to update the examples to Python3, or add a new file for each example that runs on Python3

ibayer commented 7 years ago

@aaossa Sorry for the late response. Your snipped looks good, can you open a PR?

update the examples to Python3

They should be Python3 compatible. Travis-ci tests with

    - TRAVIS_PYTHON_VERSION="2.7"
    - TRAVIS_PYTHON_VERSION="3.4"
    - TRAVIS_PYTHON_VERSION="3.5"
aaossa commented 7 years ago

@ibayer PR opened ( #112 ). I didn't see the Travis tests, thanks for mention the file.