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

Setup refresh #814

Merged
merged 4 commits into from
Apr 8, 2021
Merged

Setup refresh #814

merged 4 commits into from
Apr 8, 2021

Conversation

arcondello
Copy link
Member

@arcondello arcondello commented Apr 8, 2021

Significantly rework/simplify the setup script.

Closes #678

@arcondello arcondello requested a review from randomir April 8, 2021 18:22
@arcondello arcondello force-pushed the setup-refresh branch 2 times, most recently from 5f2bd9a to bce20df Compare April 8, 2021 18:41
@arcondello arcondello force-pushed the setup-refresh branch 3 times, most recently from 1a6690a to f80bf25 Compare April 8, 2021 19:12
Copy link
Member

@randomir randomir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

setup.cfg Outdated
License :: OSI Approved :: Apache Software License
Operating System :: OS Independent
Programming Language :: Python :: 3 :: Only
Programming Language :: Python :: 3.5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not anymore.

pyproject.toml Outdated
@@ -0,0 +1,8 @@
[build-system]
requires = [
"setuptools",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd lower-bound setuptools to avoid some issues we had in past. Although, I can't remember what those were, but I remember it was something. 😄 Environment markers, right?

Copy link
Member Author

@arcondello arcondello Apr 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set it to >=42.0.0 for now. A bit arbitrary but that's the version that started supporting pep518 so seemed natural :) I have no doubt that we will want to tighten the bound approximately 10 min after I merge.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've found the lower-bound I had in mind -- looks like it's 31.0 due to namespace packages support.

OTOH, I'm not sure where is 42.0.0 coming from? Looking at pypa/setuptools#651, seems like PEP-518 support was not needed/not implemented in setuptools.

Copy link
Member

@randomir randomir May 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then again, I see from pypa/setuptools@89155ab the full PEP-420 support (native namespace packages) is only available since 40.1.0.

We're not using them (yet), but it seems prudent to use setuptools that supports them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the number came from their changlog.

@arcondello arcondello merged commit f45da82 into dwavesystems:main Apr 8, 2021
@arcondello arcondello deleted the setup-refresh branch April 8, 2021 19:50
arcondello added a commit that referenced this pull request May 7, 2021
New Features
------------
* Support PEP518 for building the package #814
* Add `cyVariables.size()` and `.at(..)` to improve cython access to the `Variables` object
* Add new `QuadraticModelBase` and `BinaryQuadraticModel` implementation in c++ #818, #819

Fixes
-----
* Fix broken documentation links #815
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.

Specify build enivornment by PEP 518 standard
2 participants