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.py: remove pip import #49

Merged
merged 2 commits into from Oct 3, 2022
Merged

setup.py: remove pip import #49

merged 2 commits into from Oct 3, 2022

Conversation

abn
Copy link
Contributor

@abn abn commented May 26, 2022

This change removes an unused import of pip from setup.py in order to make the project compatible with default setuptools build via PEP 517 front ends.

If pip is indeed required, this needs to be explicitly specified in a pyproject.toml file as shown below to ensure build systems do not fail unexpectedly.

[build-system]
requires = ["setuptools", "pip"]
build-backend = "setuptools.build_meta"

abn added 2 commits May 26, 2022 16:37
This change removes an unused import of pip from setup.py in order to 
make the project compatible with default setuptools build via PEP 517
front ends.
@abn
Copy link
Contributor Author

abn commented May 26, 2022

@boisgera this came up through a poetry user in our community. Just wanted to ensure the project is build-able in isolated environments.

Let me know if you need any specific changes.

@boisgera
Copy link
Owner

Hi @abn !

You're 100% right about the pip dependency. It is not required (anymore), only setuptools is needed.
And the setuptools explicit dependency in the pyproject.toml won't hurt either 😊

For the second commit, that tweaks the CI, I have two concerns (if I understand the change):

  • It runs on all branches, but so far I only test master (the tests probably fail on the other -- experimental or deprecated -- branches). Would it make sense to restrict it to master only?

  • Among the 3 CI actions, the linux ones performs the tests on linux but also build the documentation and deploys it to gh-pages. I guess that we don't want that to happen right? (The github action is triggered as soon as a PR is submitted?). I should probably split the linux action in two and trigger only the test part on a PR.

What do you think? If we agree, please change the target of the action to master and I'll accept the PR and make the other changes to the actions afterwards.

Thanks for your help!

Cheers,

Sébastien

@MartinBernstorff
Copy link

MartinBernstorff commented Sep 27, 2022

Hi boisgera! This was causing us quite a bit of headache, thanks for taking a look at it!

I'd love to make the changes requested but don't have edit rights for the PR. Would you prefer that you changed it, or I submit a new PR?

@boisgera
Copy link
Owner

boisgera commented Oct 3, 2022

Hi boisgera! This was causing us quite a bit of headache, thanks for taking a look at it!

I'd love to make the changes requested but don't have edit rights for the PR. Would you prefer that you changed it, or I submit a new PR?

I'll merge the PR and adjust it afterwards, that's probably the easiest way.
Cheers,
SB

@boisgera boisgera merged commit 99e85d2 into boisgera:master Oct 3, 2022
@boisgera
Copy link
Owner

boisgera commented Oct 3, 2022

... and I actually have nothing to change, since @abn has already done everything (and I didn't even notice ...).
Thanks a lot ! 👍

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.

None yet

3 participants