move setup.cfg to pyproject.toml by sjvrijn · Pull Request #352 · NLeSC/python-template

sjvrijn · 2023-09-28T16:00:10Z

Description

Removed setup.py and setup.cfg
Added all relevant info to pyproject.toml

Some minor adjustments made during the translation:

Added the requirement >=64.0.0 to setuptools under [build-system]: this is the first version to add support for PEP660: Editable installs for pyproject.toml based builds
Remove obsolete zip_safe setting
Simplified package data to just include-package-data = true under [tool.setuptools] in combination with requiring setuptools-scm. This automatically adds all data files that are included under source control.
Replaced package find options with an explicit packages = {{ cookiecutter.package_name }} under [tools.setuptools]
Added an additional cookiecutter variable repository_url to define an https github url under [project.urls], since it wouldn't accept the git url in cookiecutter.repository:

ValueError: invalid pyproject.toml config: `project.urls.Repository`.
      configuration error: `project.urls.Repository` must be url

Related issues:

Use pyproject.toml only for installing the generated package #351

Instructions to review the pull request

Create a python-template-test repo on GitHub (will be overwritten if existing)

cd $(mktemp -d --tmpdir py-tmpl-XXXXXX)
cookiecutter -c pyproject-toml-351 https://github.com/NLeSC/python-template
# Fill with python-template-test info
cd python-template-test
git init
git add --all
git commit -m "First commit"
git remote add origin https://github.com/<you>/python-template-test
git push -u origin main -f
python -m venv env
source env/bin/activate
python -m pip install --upgrade pip setuptools
python -m pip install '.[dev,publishing]'

gcroci2

Much cleaner now, great :) I inserted few minor suggestions, and I would only reorder the sections a bit (e.g., [tool.setuptools.packages.find] should go under [tool.setuptools] section).

gcroci2 · 2023-10-05T08:20:35Z

{{cookiecutter.directory_name}}/pyproject.toml

+    "tox",
+    "myst_parser",
+]
+publishing = [


In my project I have build package as well in the publishing extra. Should be needed, right?

It's not yet needed, since we still depend on the old build methods. See #318 and #328 for two ways to move forward on this. Needs a separate PR by someone brave enough to make a choice ;)

{{cookiecutter.directory_name}}/pyproject.toml

egpbos · 2023-10-13T13:36:36Z

{{cookiecutter.directory_name}}/pyproject.toml

After reading up on setuptools package discovery and data file inclusion (check especially the comment in the pyproject.toml example) I think this whole section could be removed. For sure include-package-data = true can be removed, because it is now true by default. The package discovery could be removed if we move to a src-based layout.

This actually has two advantages:

This above, i.e. simpler configuration.

It also automatically supports single-file packages (you don't need a folder for a package if it only consists of one file, but to support that you normally need to reconfigure, see e.g. how I did that in kwandl; I think that with a src dir, though, that will also just be autodiscovered, yay!).

Suggested change

[tool.setuptools]

include-package-data = true

packages = ["{{ cookiecutter.package_name }}"]

To be clear, this suggestion would have to be accompanied by adding a src dir.

I also agree that moving to a src-based layout would be the nicest way to go :)

egpbos · 2023-10-13T13:57:52Z

This is a great PR, thanks for this effort!

One additional change to a src-based layout would make this even better, since it would simplify configuration (see my comments). Do you have time for this? If not, I think we can merge after taking @gcroci2's suggestions into account and then make a new issue/PR for moving to src-based.

sjvrijn · 2023-10-17T15:40:11Z

Thanks for the feedback @egpbos and @gcroci2, the PR has been updated.
Given the difficulties already indicated in #173, I've chosen to keep the move to an src setup to a separate PR that I'll work on next.

gcroci2

There is the same typo (almost) everywhere: reporitory instead of repository. For the rest feel free to merge when it's fixed :)

{{cookiecutter.directory_name}}/.github/next_steps/01_sonarcloud_integration.md

{{cookiecutter.directory_name}}/.github/next_steps/02_citation.md

{{cookiecutter.directory_name}}/.github/next_steps/03_readthedocs.md

{{cookiecutter.directory_name}}/README.md

{{cookiecutter.directory_name}}/sonar-project.properties

sjvrijn · 2023-10-17T17:32:16Z

@gcroci2 good catch! downside of making a typo during a 'find & replace all' 😅

egpbos · 2023-10-17T19:07:27Z

@sjvrijn great stuff, keep it coming! 😄 Be aware that the failure with >=3.9 is already fixed in #347.

sjvrijn · 2023-10-17T19:36:36Z

@egpbos All other tests are now fixed, I'll merge this after #347 (and #353? I guess)

egpbos · 2023-10-17T21:02:02Z

Ok, both other PRs are merged. There are some merge conflicts left to fix. Also, I was wondering: you now invoke -m build instead of setup.py, but doesn't that actually need build installed as a dependency (as @gcroci2 earlier suggested, see #352 (comment))?

Finally, if you are up for becoming a true UGHist, I warmly invite you to squash some of the commits (happy to do it together, I'm at the office tomorrow), but no worries if you don't have the time 😁

sjvrijn · 2023-10-19T10:35:56Z

@egpbos Yep, I indeed had to include build to enable the sdist build according to this StackOverflow question. Unless there's another way you know of?

Also, UGH challenge accepted 💪 I've rebased my commits on top of the merged main, and fixuped some of the corrective commits into the first one.

egpbos · 2023-10-19T17:39:29Z

UGH! Super nice addition, thanks!

This commit changed the test_subpackage build command, but left the now unnecessary '--sdist' and '--wheel' parameters. They are on by default for `python -m build`

sjvrijn requested review from apalha, egpbos and gcroci2 and removed request for apalha September 28, 2023 17:02

gcroci2 approved these changes Oct 5, 2023

View reviewed changes

egpbos reviewed Oct 13, 2023

View reviewed changes

egpbos linked an issue Oct 13, 2023 that may be closed by this pull request

Use pyproject.toml only for installing the generated package #351

Closed

sjvrijn mentioned this pull request Oct 17, 2023

wrong links that point towards the github page of the new package #355

Closed

gcroci2 approved these changes Oct 17, 2023

View reviewed changes

sjvrijn and others added 3 commits October 19, 2023 01:31

move setup.cfg to pyproject.toml

4d32164

make bumpversion check pyproject.toml instead of setup.cfg

53a6999

remove setup.py reference from subpackages test

3d9889c

sjvrijn force-pushed the pyproject-toml-351 branch 2 times, most recently from 7656b95 to 3d9889c Compare October 19, 2023 10:06

egpbos merged commit d1086f8 into main Oct 19, 2023

egpbos added a commit that referenced this pull request Oct 19, 2023

small fixup of #352

ad1ebf0

This commit changed the test_subpackage build command, but left the now unnecessary '--sdist' and '--wheel' parameters. They are on by default for `python -m build`

egpbos mentioned this pull request Oct 20, 2023

use python -m build to build #358

Merged

egpbos added a commit that referenced this pull request Oct 20, 2023

small fixup of #352

375ea62

This commit changed the test_subpackage build command, but left the now unnecessary '--sdist' and '--wheel' parameters. They are on by default for `python -m build`

sjvrijn deleted the pyproject-toml-351 branch October 27, 2023 20:37

gcroci2 mentioned this pull request Nov 15, 2023

remove leftover setup.cfg refs #360

Closed

egpbos mentioned this pull request Dec 20, 2023

ssh instead of https URL in setup.cfg #350

Closed

sjvrijn mentioned this pull request Feb 19, 2024

Update Python Packaging section to use pyproject.toml instead of setup.cfg NLeSC/guide#326

Closed

	[tool.setuptools]
	include-package-data = true
	packages = ["{{ cookiecutter.package_name }}"]

Conversation

sjvrijn commented Sep 28, 2023

Uh oh!

gcroci2 left a comment

Choose a reason for hiding this comment

Uh oh!

gcroci2 Oct 5, 2023

Choose a reason for hiding this comment

Uh oh!

egpbos Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

egpbos Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

gcroci2 Oct 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

egpbos commented Oct 13, 2023

Uh oh!

sjvrijn commented Oct 17, 2023

Uh oh!

gcroci2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sjvrijn commented Oct 17, 2023

Uh oh!

egpbos commented Oct 17, 2023

Uh oh!

sjvrijn commented Oct 17, 2023

Uh oh!

egpbos commented Oct 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sjvrijn commented Oct 19, 2023

Uh oh!

egpbos commented Oct 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

gcroci2 Oct 17, 2023 •

edited

Loading

egpbos commented Oct 17, 2023 •

edited

Loading