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

Initial commit of pyproject-toml branch #201

Closed
wants to merge 1 commit into from
Closed

Initial commit of pyproject-toml branch #201

wants to merge 1 commit into from

Conversation

stevenrhall
Copy link

This branch moves much of the configuration of the cyipopt package to pyproject.toml. My motivation for this is that as currently configured, cyipopt is incompatible with poetry. Trying to do

poetry add cyipopt

results in an error from poetry

...
Note: This error originates from the build backend, and is likely not a problem with poetry but with 
cyipopt (1.2.0) not supporting PEP 517 builds. You can verify this by running 
'pip wheel --use-pep517 "cyipopt (==1.2.0)"'.

To fix this, I moved the dependencies (and much of the other setup information) into pyproject.toml. In the process, that allows deletion of the AUTHORS, MANIFEST.in, pytest.ini, and requirements.txt files, without losing any information.
In addition, it greatly simplifies the setup.py script, which now only has to generate the extension modules.

BTW, I don't think this is just a poetry problem. In the past, I've had trouble installing cyipopt because of incompatible dependencies.

You can test that the new fork works as intended in poetry by executing

pip install git+https://github.com/stevenrhall/cyipopt.git@pyproject-toml 

in a Python environment, or

poetry add git+https://github.com/stevenrhall/cyipopt.git@pyproject-toml 

using poetry, and then in either case running the cyipopt test suite. I've also used the new fork in a Github workflow successfully.

This branch moves much of the configuration of the cyipopt package to
pyproject.toml. In particular, the dependencies are properly accounted
for so that cyipopt can be used in a poetry environment. It should also
reduce failures of `pip install cyipopt` that I have encountered. In 
addition, it greatly simplifies the setp.py script, which now only
has to generate the extension modules.
@stevenrhall
Copy link
Author

Sorry, I should have titled the pull request more appropriately!

@moorepants
Copy link
Collaborator

Thank you for your interest, but I'm not keen on this addition. I plan to support a setup.py based installation until it is actually disabled by Python. cyipopt should be pip installable and, as far as I understand, it is. In the next release we hope to provide at least linux wheels and that may ease installation with some other package managers, such as poetry. For the record, cyipopt cannot be installed by poetry no matter how much work is done on this toml file. Building cyipopt requires a number of dependencies that are not available on PyPI and thus aren't installable by poetry, so it isn't in our interest to make special cases for this package manager when it causes issues with the primary package managers we work to support: pip and conda.

If you want to install your package with poetry, there is a simple work around:

# first install all the OS specific build dependencies not available on pypi (see installation instructions)
poetry add setuptools cython numpy
poetry add cyipopt

More information can be found here: #167

Thanks again for the contribution, but I am going to close this PR.

@moorepants moorepants closed this Apr 11, 2023
@stevenrhall
Copy link
Author

I understand your closing the PR, and that's fine.

Your workaround above does not work, at least on MacOS. As roughly per your comment in issue #167, on my system with the Ipopt dependency already installed, I do this:

poetry new cyipopt_install
cd cyipopt_install/
poetry add setuptools cython numpy
poetry add cyipopt

I get the failure message in my earlier comment. (Not sure if the workaround your suggested in #167 only works on ubuntu, or whether it's a change in poetry or its dependencies since then.)

The workaround that does work for now is to instead

poetry add git+https://github.com/mechmotum/cyipopt

or better

poetry add git+https://github.com/mechmotum/cyipopt@d67f353f9b5cb2e7f6d1782f14b5422b733e2830

The hash is the merge from pull request #162, which is the first 1.3.0.dev0 merge to master to have a pyproject.toml file.

Thanks BTW for your work on cyipopt. I've been using it for quite some time (maybe since soon after it was forked).

@stevenrhall stevenrhall deleted the pyproject-toml branch April 11, 2023 14:53
@moorepants
Copy link
Collaborator

I get the failure message in my earlier comment. (Not sure if the workaround your suggested in #167 only works on ubuntu, or whether it's a change in poetry or its dependencies since then.)

Ok, we can look into this. Thanks for letting me know.

Thanks BTW for your work on cyipopt. I've been using it for quite some time (maybe since soon after it was forked).

Much appreciated.

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.

3 participants