NLeSC / python-template

Netherlands eScience Center Python Template
https://research-software-directory.org/software/nlesc-python-template
Apache License 2.0
163 stars 73 forks source link

move setup.cfg to pyproject.toml #352

Closed sjvrijn closed 9 months ago

sjvrijn commented 9 months ago

Description

Some minor adjustments made during the translation:

Related issues:

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]'
egpbos commented 9 months ago

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 commented 9 months ago

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.

sjvrijn commented 9 months ago

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

egpbos commented 9 months ago

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

sjvrijn commented 9 months ago

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

egpbos commented 9 months ago

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 https://github.com/NLeSC/python-template/pull/352#discussion_r1347001242)?

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 commented 9 months ago

@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 commented 9 months ago

UGH! Super nice addition, thanks!