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

Migrate to pyproject.toml #1668

Merged
merged 11 commits into from
Aug 1, 2024
Merged

Migrate to pyproject.toml #1668

merged 11 commits into from
Aug 1, 2024

Conversation

lucc
Copy link
Collaborator

@lucc lucc commented Jul 31, 2024

The plan behind this PR is to migrate as much info as possible from setup.* to pyproject.toml.

We can then decide if we want to support setuptools or poetry or both as our build system.

lucc added 3 commits July 31, 2024 11:08
The github actions configuration is under .github/workflows/ and tox is
currently not used.
@lucc lucc added deployment dependencies Pull requests that update a dependency file labels Jul 31, 2024
@lucc lucc self-assigned this Jul 31, 2024
@pazz
Copy link
Owner

pazz commented Jul 31, 2024

Ah cool, you beat me to it, I was just doing that but it takes a while to catch up on the docs :)

@pazz
Copy link
Owner

pazz commented Jul 31, 2024

I was going to propose to remove setup.* and poetry configs alltogether, and keep the nix config under extra/.

About the pyproject.toml file, here is what I cam up with atm:

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

[project]
name = "alot"
authors = [
    {name = "Patrick Totzke", email = "patricktotzke@gmail.com"},
]
description = "Terminal MUA using notmuch mail"
readme = "README.md"
requires-python = ">=3.8"
classifiers=[
	'Development Status :: 4 - Beta',
	'Environment :: Console :: Curses',
	'Intended Audience :: End Users/Desktop',
	'Programming Language :: Python :: 3 :: Only',
	'Topic :: Communications :: Email :: Email Clients (MUA)',
	'Topic :: Database :: Front-Ends',
        'License :: OSI Approved :: GNU General Public License v3 or later (GPLv3+)',
]
dependencies = [
    'notmuch2>=0.1',
    'urwid>="1.3.0"',
    'urwidtrees>="1.0.3"',
    'twisted>="24.3.0"',
    'python-magic',
    'configobj>="4.7.0"',
    'gpg>"1.10.0"'
]
version= "0.11.0"


[project.scripts]
alot = "alot.__main__:main"

[project.urls]
Repository = "https://github.com/pazz/alot"
Documentation = "https://alot.readthedocs.io/en/latest/"
Issues = "https://github.com/pazz/alot/issues"

Left todo here are

  • optional dependencies, possibly for groups "dev" and "doc" as you had with poetry
  • configuring setuptools to use a dynamic version string from git would be cool
  • you should really be listed as author or maintainer to get credit for your work

@lucc
Copy link
Collaborator Author

lucc commented Jul 31, 2024

Yea I am also fighting the docs and working my way forward in small commits. I noticed that we import some data from alot/__init_.py into setup.py which is also reused in docs/conf.py.

@pazz
Copy link
Owner

pazz commented Jul 31, 2024

I think we can hardcode the version string for now and do dynamic version strings in a separate PR, possibly after a release. It seems that setuptools can do this in two ways:

  1. based on a package attribute (currently alot.__version__) like this
[project]
dynamic = ["version"]
[tool.setuptools.dynamic]
version = {attr = "alot.__version__"}
  1. based on git tags/commit, using a separate tool that is part of setuptools

It seems that there is an easy way to do 2) and get the version string into sphinx and readthedocs without dupication.

build-backend = "setuptools.build_meta"


[tool.poetry]
Copy link
Owner

Choose a reason for hiding this comment

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

whoopsie, this seems to have broken the nix stuff...
The idea was to remove all poetry related configs and keep this file vanilla setuptools.
Would it suffice to adjust the nix script to reflect the namespace change?

I'll stop interfering with your branch now, sorry :)

@pazz pazz changed the title Migrate to pypeoject.toml Migrate to pyproject.toml Jul 31, 2024
@pazz
Copy link
Owner

pazz commented Jul 31, 2024

FYI: this branch is based off yours here and implements the dynamic versioning and metadata:
(feel free to incorporate)

@lucc
Copy link
Collaborator Author

lucc commented Jul 31, 2024

I see your argument that you lean towards setuptool vs portry because it is the more established tool.

One advantage of poetry I like is the lock file. It makes the setup reproducible. No idea if and how setuptools can do this. (I know there is the requirements.yml file but is it as exact as the portry lock file?).

Then again, if we advise our developers to create virtualenvs with --system-packages we might break reproduciblilty anyway.

lucc and others added 5 commits August 1, 2024 00:10
This still keeps poetry as the default build backend.
There seems to be a race condition when running this cleanup on github
actions which results in
> FileNotFoundError: [Errno 2] No such file or directory: 'S.gpg-agent.ssh'
@lucc
Copy link
Collaborator Author

lucc commented Aug 1, 2024

@pazz contrary to my initial comment I now implemented the switch from poetry to setuptools completely in this branch.

I think it is ready from my side. Can you test it on debian or some other non nix system. I am using nixos and that saves me from all the gpg>1.10 trouble.

@lucc lucc marked this pull request as ready for review August 1, 2024 07:13
@lucc
Copy link
Collaborator Author

lucc commented Aug 1, 2024

@pazz I only included one commit from your "dynamic-version" branch because the rest was a bit broken. If you rebase the branch on this branch now (or on master, I think we can merge) then you should be able to fix it.

@pazz
Copy link
Owner

pazz commented Aug 1, 2024

One advantage of poetry I like is the lock file. It makes the setup reproducible. No idea if and how setuptools can do this. (I know there is the requirements.yml file but is it as exact as the portry lock file?).

In principle I agree though, but personally I have never seen the actual benefit of reproducible setups in practice IIRC. No string feelings wither way really, but if poetry ultimately depends on setuptools and is one more step to do when setting things up from scratch I would prefer to cut it out.

Then again, if we advise our developers to create virtualenvs with --system-packages we might break reproduciblilty anyway.

yes, true.

I think the PR as is is almost ready, except perhaps we want to leave the last two commits about version strings out?
I agree it makes sense to keep that separately anyway and I do not know of the effect of using dynamic version strings for the package but then not in alot/ or docs/. It will leave things inconsistent for sure (alot.__init__.py defines the version string used in different places, and not sure how future proof the hardcoded version string in the nix file is.

@lucc
Copy link
Collaborator Author

lucc commented Aug 1, 2024

I removed the last two commits.

Sitenote

About the different python build backends and frontends I read a bit of history about distutils and setuptools and pip somewhere but I can't remember where. My uptake was that there are two tasks that need to be solved:

  1. packing files into a "distribution package" (wheel, sdist, bdist, ...)
  2. a nice tool for developers and users to manage dependencies, environments, tests, installs, uploads to pypi ...

Poetry is for 2. and setuptools was for both but tries to stay in 1. in the future I think. They try to be a library for 1. and deprecate the command line use case for 2. (which was python setup.py ...).

The section [build-system] in pyproject.toml reflects that. When we switch from poetry to setuptools it changed thus:

 [build-system]
-requires = ["poetry-core"]
-build-backend = "poetry.core.masonry.api"
+requires = ["setuptools", "setuptools-scm"]
+build-backend = "setuptools.build_meta"

This section allows tools from point 2 above to select the correct library for point 1 above. See also the Motivation of PEP 621.

Or a longer overview / introduction for developers how want to select a backend/frontend for their python project:
https://www.pyopensci.org/python-package-guide/package-structure-code/python-package-build-tools.html

@lucc lucc requested a review from pazz August 1, 2024 09:05
Copy link
Owner

@pazz pazz left a comment

Choose a reason for hiding this comment

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

lgtm

@lucc lucc merged commit a7bf032 into master Aug 1, 2024
18 checks passed
@lucc lucc deleted the pyproject branch August 1, 2024 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants