Skip to content
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

Python 3 support for CyLP #28

Merged
merged 22 commits into from
Jun 30, 2019
Merged

Python 3 support for CyLP #28

merged 22 commits into from
Jun 30, 2019

Conversation

jjhelmus
Copy link
Contributor

@jjhelmus jjhelmus commented Oct 2, 2015

  • Changes needed for CyLP to work in Python 3.4.
  • A single code base is used which supports Python 2.7, 3.3, and 3.4.
  • Unit test pass in all of the above Python version.

closes #15

Python 3 does not differentiate betten int and long so the PyInt_
functions must be replaced with PyLong_.

Macros from cx_Oracle project.
The izip function fron itertools was removed from Python 3, use of zip is
recommended.
TypeError and NotImplementedError are build ins
long was removed in Python 3, use int in its place.
The build in reduce function was removed in Python 3 and must be imported
from functools.
Due to the different methods that Python 2 and 3 represent strings
it is necessary to encode the filename passed to the readMps function
which can then be converted into a char* pointer for use in C.

Since strings do not share a type in Python 2 and 3 the filename argument
of the readMps function must not be typed.
@sebastroy
Copy link

1b798eb has a mistake: it's missing a parenthesis at the end of the line:

         print(0.5 * x * (x * G).T + np.dot(c, x) - self.objectiveOffset

in file QP.py

in the same commit in QPGen.py on the very first line it's missing a "t" to import:

         from __future__ impor print_function

I also get an error when executing "python setup.py install" (but I have it with pip install too...):
writing cylp.egg-info/PKG-INFO
Traceback (most recent call last):
File "setup.py", line 418, in
package_data={"cylp": ['cpp/.cpp', 'cpp/.hpp', 'cpp/*.h', 'VERSION']},)
File "/usr/lib/python3.5/distutils/core.py", line 148, in setup
dist.run_commands()
File "/usr/lib/python3.5/distutils/dist.py", line 955, in run_commands
self.run_command(cmd)
File "/usr/lib/python3.5/distutils/dist.py", line 974, in run_command
cmd_obj.run()
File "setup.py", line 392, in run
install.run(self)
File "/usr/lib/python3.5/site-packages/setuptools/command/install.py", line 65, in run
orig.install.run(self)
File "/usr/lib/python3.5/distutils/command/install.py", line 551, in run
self.run_command(cmd_name)
File "/usr/lib/python3.5/distutils/cmd.py", line 313, in run_command
self.distribution.run_command(command)
File "/usr/lib/python3.5/distutils/dist.py", line 974, in run_command
cmd_obj.run()
File "/usr/lib/python3.5/site-packages/setuptools/command/install_egg_info.py", line 33, in run
self.run_command('egg_info')
File "/usr/lib/python3.5/distutils/cmd.py", line 313, in run_command
self.distribution.run_command(command)
File "/usr/lib/python3.5/distutils/dist.py", line 974, in run_command
cmd_obj.run()
File "/usr/lib/python3.5/site-packages/setuptools/command/egg_info.py", line 173, in run
writer(self, ep.name, os.path.join(self.egg_info, ep.name))
File "/usr/lib/python3.5/site-packages/setuptools/command/egg_info.py", line 388, in write_pkg_info
metadata.write_pkg_info(cmd.egg_info)
File "/usr/lib/python3.5/distutils/dist.py", line 1107, in write_pkg_info
self.write_pkg_file(pkg_info)
File "/usr/lib/python3.5/distutils/dist.py", line 1128, in write_pkg_file
long_desc = rfc822_escape(self.get_long_description())
File "/usr/lib/python3.5/distutils/util.py", line 471, in rfc822_escape
lines = header.split('\n')
TypeError: a bytes-like object is required, not 'str'

The culprit is the s_README encoding. I just commented it out from setup.py but I guess the encoding from the getBdistFriendlyString function should be fixed instead. Thanks!

@jjhelmus
Copy link
Contributor Author

Good catch on those mistakes @sebastroy, I added a commit to fix them. Investigating the install issue.

@sebastroy
Copy link

Great thanks!

@jjhelmus
Copy link
Contributor Author

Commit 13d5619 seem to fix the install issue although it might have broken the bdist_mpkg and bdist_wininst commands in setup.py. With commit 89ce697 and 7fd344c I am able to run python setup.py bdist_mpkg on my machine in Python 2.7 and Python 3.5 but it fails with lines concerning pax failing:

pax: Cpio header field is too small to store file ./cylp/VERSION
pax: Cpio header field is too small to store file ./cylp-0.7.4-py2.7.egg-info
pax: Cpio header field is too small to store file ./cylp-0.7.4-py2.7.egg-info/dependency_links.txt
pax: Cpio header field is too small to store file ./cylp-0.7.4-py2.7.egg-info/not-zip-safe
pax: Cpio header field is too small to store file ./cylp-0.7.4-py2.7.egg-info/PKG-INFO
pax: Cpio header field is too small to store file ./cylp-0.7.4-py2.7.egg-info/requires.txt
pax: Cpio header field is too small to store file ./cylp-0.7.4-py2.7.egg-info/SOURCES.txt
pax: Cpio header field is too small to store file ./cylp-0.7.4-py2.7.egg-info/top_level.txt
error: command '/bin/pax' failed with exit status 1

Unfortunately my knowledge of bdist_mpkg is limited and I have no idea where to begin to debug this. I get the same error messages on the current master so this may be limited to my machine.

I have no way of easily testing the bdist_wininst command.

@sebastroy
Copy link

I'm on linux myself... If I replace the call to getBdistFriendlyString and use plain "u" like the other two lines, install works but it possibly breaks the 2.7 version... In a nutshell, the issue is under python 2, str = bytes and under 3, bytes = bytes.

@jjhelmus
Copy link
Contributor Author

jjhelmus commented Feb 9, 2016

Wanted to bump this PR and see if there was anything blocking this from being merged. I am very interested in having a Python 3 version of CyLP and am willing to fix up any outstanding issues.

@sebastroy
Copy link

Fine by me

@scollis
Copy link
Contributor

scollis commented Nov 29, 2016

Ping.. I am almost entirely 3 now.. Would be great to see this in the master branch

@RhysU
Copy link

RhysU commented Apr 18, 2017

Ditto on a desire to see this in master.

@Guzzii
Copy link

Guzzii commented May 2, 2017

It would be really great to see python3 support from CyLP

@scollis
Copy link
Contributor

scollis commented Jul 13, 2017

Are we going to see action on this?

@SRserves85
Copy link

Bump, Is this ever happening?

@tkralphs
Copy link
Member

I would still like to get back to CyLP and spend some time addressing the backlog of PRs, but I have obviously lost credibility in claiming this will happen. I will try to find a weekend soon when I can play with it. Obviously, it is not difficult to merge the PR. The time consuming part is testing. If those who have been using CyLP with Python 3 can post and say what their experience has been, it would be helpful in getting a level of comfort.

@sschnug
Copy link

sschnug commented Jan 29, 2018

I did not use this pull-request extensively, but i never approached problems!

While documentation and (mostly) missing development is depressing, it's still a very interesting project and official python3 support would be great!

(Two examples: footrule-ranking (Clp), kemeny-ranking (Cbc))

@scollis
Copy link
Contributor

scollis commented Jan 29, 2018

We are now just redirecting folks for Py-ART to use the @jjhelmus branch. Having master fork updated with jjhelmus/py3 would be great

@scollis
Copy link
Contributor

scollis commented Feb 10, 2018

All, and specifically @tkralphs , We are at the point now that I am considering creating a fork with @jjhelmus 's work, making his py3 branch master and using that for Py-ART. This is a great project by we need a Py3 compatible master branch

@tkralphs
Copy link
Member

I don't think a fork would be in anyone's best interest. Although this is not exactly my project, I do have the ability to add collaborators and I think I said at some point that I would be happy to hear from anyone who is willing to help, though maybe that got lost in the fray. The implication of the above comment is that there are people who would be happy to help. I would want to have some discussions with those people to make sure we're on the same page, but this seems to be the way forward.

@scollis
Copy link
Contributor

scollis commented Feb 21, 2018

Ok! I can help.. It looks like we have some conflicts.

@scollis
Copy link
Contributor

scollis commented Feb 21, 2018

@jjhelmus was it just Py3 compatibility issues or did you refactor? I am wondering if I should start with the current master and work from that or try to resolve the conflicts.

@scollis
Copy link
Contributor

scollis commented Feb 21, 2018

Actually now I look at the commits it looks quite involved.

@jjhelmus
Copy link
Contributor Author

I created this patch over two years, I do not recall what all was done.

Unfortunately, I do not have the time to re-work the patches or otherwise move this forward. I'd be happy for someone else to pick up this work but I cannot offer any help myself.

@scollis
Copy link
Contributor

scollis commented Feb 21, 2018

No worries and understand @jjhelmus

@tkralphs
Copy link
Member

As there haven't been very many commits since this PR was created and such commits were not very involved, I hope that resolving the conflicts would be not be too difficult. @scollis, perhaps we can schedule a face-to-face meeting and discuss a strategy? It would certainly be helpful to have someone who's using CyLP regularly and has a little bandwidth to push this forward. I'll actually be using CyLP with students in the next month or two and I'm sure they will be wanting to use Python 3, so between the two of us, we should be able to get this going.

@sschnug sschnug mentioned this pull request Apr 16, 2018
@SteveDiamond
Copy link

Hi everyone, is CyLP a live project? I'm a developer of cvxpy, and we use CyLP to interface to Cbc. It's an important tool for us, so if we can help out in modernizing CyLP let us know. I've posted here because Python 3 support is essential.

@sebastroy
Copy link

My rule of thumb for "dead" projects is last commit more than 4-6mo ago.

For what it's worth, we are using ortools now. It was a bit clunky a few years ago but it came a long way since then and is very stable. It does not have 100% all the cbc features (neither does CyLP for that matter) but it had enough for us. Unlike pulp, it does interface with the coin-or library rather than launch cbc in a shell wrapped in python code.

@scollis
Copy link
Contributor

scollis commented Mar 13, 2019

Hey @SteveDiamond We still use it.. I have been very slack in taking @tkralphs on his offer.. Having enough trouble with Py-ART which uses CyLP.

@scollis
Copy link
Contributor

scollis commented Mar 13, 2019

What would be amazing is a LP code in Scipy.optimize or suchlike

@tkralphs
Copy link
Member

I'm still here and still hoping for a co-maintainer :). Although I have zero credibility left in terms of promises, I am going to be using CyLP myself in one of my classes this semester and will be trying to do some maintenance work over the next couple of weeks.

@tkralphs tkralphs self-assigned this Mar 13, 2019
@tkralphs tkralphs mentioned this pull request Mar 22, 2019
@tkralphs
Copy link
Member

I just re-generated the Cython files and updated this PR in #67.

@tkralphs
Copy link
Member

I posted a beta version with Python 3 support to Pypi:

pip install --pre cylp

There is a binary wheel for Windows, but you still need to build Cbc before installing on Linux and OS X.

@tkralphs
Copy link
Member

I seem to have figured out bug that prevented Cython 0.29 from properly re-generating the files. I re-generated the files once again with Cython 0.29 and just pushed a binary wheel up for Python 3.7 on Windows. With a little more testing, I'll be ready to merge and release an official Python 3 version.

@tkralphs tkralphs merged commit 7fd344c into coin-or:master Jun 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compilation problems with python 3.4
9 participants