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

chore: move metadata to pyproject.toml #236

Merged
merged 4 commits into from
Apr 2, 2023
Merged

chore: move metadata to pyproject.toml #236

merged 4 commits into from
Apr 2, 2023

Conversation

SauravMaheshkar
Copy link
Contributor

pyproject.toml is the new standard for packaging python packages (first introduced in PEP 518 and later expanded in PEP 517, PEP 621 and PEP 660). This PR proposes to switch the metadata declaration from the legacy setup.cfg in favour of pyproject.toml. This would also allow for a better minimal project structure.

@SauravMaheshkar
Copy link
Contributor Author

@nkoep Request for Review

Copy link
Member

@nkoep nkoep left a comment

Choose a reason for hiding this comment

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

Hey, thanks a lot for the PR! I've been meaning to get rid of the setup.cfg file as well. I just added a bunch of pedantic style comments. If you don't mind, could you also change all indents to 2 spaces for consistency? 🙇

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@SauravMaheshkar
Copy link
Contributor Author

Hey, thanks a lot for the PR! I've been meaning to get rid of the setup.cfg file as well. I just added a bunch of pedantic style comments. If you don't mind, could you also change all indents to 2 spaces for consistency? 🙇

Hello @nkoep thanks a lot for the feedback. I addressed the requested changes in edda1b2. Looking forward to contributing more 😄

Copy link
Member

@nkoep nkoep left a comment

Choose a reason for hiding this comment

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

Thanks for the swift updates! LGTM!

@SauravMaheshkar
Copy link
Contributor Author

SauravMaheshkar commented Apr 2, 2023

@nkoep Do you think we should try and add a pip install -U pip setuptools wheel in the CI ? This repository does use the older version of setup-python in CI. I can make a quick PR to update it. Or maybe update this PR

@nkoep
Copy link
Member

nkoep commented Apr 2, 2023

@nkoep Do you think we should try and add a pip install -U pip setuptools wheel in the CI ? This repository does use the older version of setup-python in CI. I can make a quick PR to update it. Or maybe update this PR

I think this should be enough:

diff --git a/pyproject.toml b/pyproject.toml
index bcb5f7e..5f885e4 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -1,5 +1,5 @@
 [build-system]
-requires = ["pip==21.3.1", "setuptools==59.6.0", "setuptools-scm[toml]>=6.2"]
+requires = ["pip==22.3.1", "setuptools==65.6.3", "setuptools-scm[toml]>=6.2"]
 build-backend = "setuptools.build_meta"

Feel free to add it to this PR.

@SauravMaheshkar
Copy link
Contributor Author

Can I add

version = "2.1.1"

pyproject.toml Show resolved Hide resolved
@coveralls
Copy link

Coverage Status

Coverage: 87.558% (+0.1%) from 87.442% when pulling 7448b64 on SauravMaheshkar:master into cf2b850 on pymanopt:master.

@nkoep nkoep merged commit 51360d3 into pymanopt:master Apr 2, 2023
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