`pkgs/sage-conf`: Move metadata from `setup.cfg` to `pyproject.toml` by mkoeppe · Pull Request #36561 · sagemath/sage

mkoeppe · 2023-10-28T17:36:45Z

This is a trivial "chore" PR. It updates Python metadata to the latest format. No controversies about the current format are known about in the Python community. In a typical open source project, someone in a Maintainer role would open a PR and then immediately merge it, or when receiving such a PR from the outside, quickly review and merge it (examples: my PRs pytest-dev/pytest-mock#410 (merged in within 1 day), pyodide/pyodide#4472, pytest-dev/pytest-xdist#1020,
sagemath/cypari2#158, fplll/fpylll#258, polymake/JuPyMake#2, cvxpy/cvxpy#2276, sagemath/cysignals#177).

In part split out from Use meson instead of configure for conda install #36489
Part of Migrate from setup.cfg to pyproject.toml #33577
- see also pkgs/sagemath-standard: Move metadata from setup.cfg to pyproject.toml #37902

📝 Checklist

The title is concise, informative, and self-explanatory.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation accordingly.

⌛ Dependencies

Depends on pkgs/sage-conf_pypi: Repair after #36400, #36435 #36533 (which changes setup.cfg, merged here)
Depends on Switch more Python toolchain packages to use (platform-independent) wheels #36885

kiwifb · 2023-10-29T20:54:46Z

There should be links for sage_conf_conda and sage_conf_pypi or do they not show up in github?

mkoeppe · 2023-10-29T20:55:51Z

These links are already in the repo and there's no change to them on this PR.

kiwifb · 2023-10-29T20:56:16Z

Scrap that, I thought the file was brand new. It is already linked. Too early on Monday for me to review properly.

mkoeppe · 2023-10-29T20:56:29Z

Note that there was already pyproject.toml for the build-system information; here we're just adding more sections to the file.

mkoeppe · 2023-10-29T20:56:57Z

Happy Monday!

tobiasdiez · 2023-10-29T23:28:22Z

Please make sure that this change is compatible with the usage of pyproject.toml in #36489 (I think that PR uses the default config that you overwrite here).

mkoeppe · 2023-10-29T23:38:57Z

Sure, there's no reason for that implementation of sage-conf to be incompatible with this pyproject.toml.

mkoeppe · 2023-11-02T15:47:20Z

Let's get this in please

tobiasdiez · 2023-11-02T17:31:06Z

Sure, there's no reason for that implementation of sage-conf to be incompatible with this pyproject.toml.

I've tried it with #36489 and its not working as there is no _sage_conf folder anymore. I think its better to stick with the defaults (i.e. either a src or sage_conf folder containing the sources).

mkoeppe · 2023-11-02T17:32:26Z

That #36489 is not ready cannot be blamed on this PR.

kiwifb

LGTM, I do not have any unexpected issue with it in sage-on-gentoo.

mkoeppe · 2023-11-02T21:04:56Z

Thank you!

tobiasdiez · 2023-11-03T01:45:16Z

As I've said before, these lines

packages = ["_sage_conf"]
py-modules = ["sage_conf"]

are not okay for me.

(I think its also not polite to cherry pick commits from a PR and to then introduce changes that lead to conflicts with that PR)

mkoeppe · 2023-11-03T01:57:43Z

As I've said before, these lines
packages = ["_sage_conf"]
py-modules = ["sage_conf"]
are not okay for me.

Tobias, what Python packages and modules are part of the sage-conf distribution is not a matter of opinion or preference, or anything that makes sense to "negotiate". It is just a fact, which was previously declared in setup.cfg; and now it is declared in pyproject.toml.

I also know that you know that.

tobiasdiez · 2023-11-03T04:15:33Z

Matthias, you are blocking #36489 and on top of that you try to enforce your conventions via this PR. This is not a nice approach.

mkoeppe · 2023-11-03T04:22:47Z

No, Tobias, as I have just explained, what is declared in pyproject.toml "is not a matter of opinion or preference", and certainly it is not "my convention". It is the standard. Do I need to point out that you opened #33577, which asks for exactly what is done in this PR?

mkoeppe · 2023-11-04T04:34:39Z

---> sage-abuse (again)

(Edit: I've referred this incident -- abusing of your privileges as a member of the Triage team, and making false and harmful accusations -- to the sage-abuse committee.)

tobiasdiez · 2023-11-04T08:10:40Z

Matthias, please don't add a positive review label when there is a negative review. Thanks!

I respect that you don't agree with me nor that you want to discuss things with me, but that doesn't imply that my review just disappears.

mkoeppe · 2023-12-06T08:00:09Z

Lint failure is unrelated, fixed in #36822.

tornaria · 2024-04-08T12:28:27Z

Can you just set the version of setuptools in pyproject.toml?

NathanDunfield · 2024-04-08T12:55:32Z

+1: Moving from setup.cfg to pyproject.toml is a good thing. There are a dozen other uses of pyproject.toml.m4 in the Sage repo, so this PR conforms to current practice.

tornaria · 2024-04-08T13:54:10Z

+1: Moving from setup.cfg to pyproject.toml is a good thing. There are a dozen other uses of pyproject.toml.m4 in the Sage repo, so this PR conforms to current practice.

It's a regression, since we are moving from setup.cfg to pyproject.toml.m4.

There's no technical need to use m4 here. It's just a level of indirection and a build dependency that is not even stated in the build-system requires.

The solution is trivial: replace SPKG_INSTALL_REQUIRES_setuptools by the actual setuptools version constraint that is necessary for sage-conf. Why are we arguing about such a trivial review request?

tornaria · 2024-04-08T13:56:27Z

pkgs/sage-conf/pyproject.toml.m4

+    "Topic :: Scientific/Engineering :: Mathematics",
+]
+urls = {Homepage = "https://www.sagemath.org"}
+requires-python = ">=3.9, <3.13"


Can we get rid of the upper bound here? This package is relatively simple python that should not be tied to the version of python.

This allows more flexibility (e.g. to use a released sage-conf with a beta version of sagemath-standard when one is experimenting with an update of python).

No. The version bound is set for uniformity.

orlitzky · 2024-04-08T14:58:20Z

-1

Sorry it took so long, I thought this was one of the ten other disputed tickets about the same thing. This makes sage-conf depend on sage-the-distribution. We should be going the other way.

mkoeppe · 2024-04-08T16:43:08Z

It's a regression, since we are moving from setup.cfg to pyproject.toml.m4.

It's not a "regression" because there's nothing wrong with the documented design.

There's no technical need to use m4 here.

Any other synchronization mechanism would also work. But m4 is what we use. It's outside of the scope of this PR to change what we use.

It's just a level of indirection

The indirection makes it more maintainable by centralizing constraints that are the same.

and a build dependency that is not even stated in the build-system requires.

By design, after the templating with m4, pkgs/ contains standard Python distribution package source trees; not before.

The solution is trivial: replace SPKG_INSTALL_REQUIRES_setuptools by the actual setuptools version constraint that is necessary for sage-conf. Why are we arguing about such a trivial review request?

We aren't "arguing". You are demanding a change that is outside of the scope of this PR.
Artificial friction.

orlitzky · 2024-04-08T19:04:04Z

It's a regression, since we are moving from setup.cfg to pyproject.toml.m4.

It's not a "regression" because there's nothing wrong with the documented design.

It's clear that some people disagree with this statement.

It's just a level of indirection

The indirection makes it more maintainable by centralizing constraints that are the same.

It also erroneously centralizes constraints that are not the same. But more importantly (to me), it ties sage-conf to sage-the-distribution, and is confusing as hell to the... basically everyone who is expecting a plain pyproject.toml file based on experience with other python projects.

We aren't "arguing". You are demanding a change that is outside of the scope of this PR. Artificial friction.

I think we are free to vote against this PR because it further entrenches a bad design, or because we are drunk, or for any other reason.

mkoeppe · 2024-04-08T19:20:13Z

I think we are free to vote against this PR because it further entrenches a bad design, or because we are drunk, or for any other reason.

Yes, that's how voting works. But that does not stop me from reminding community members about their duty to act responsibly.

github-actions · 2024-04-09T04:06:31Z

Documentation preview for this PR (built with commit 9977681; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

mkoeppe · 2024-04-19T19:09:02Z

The current tally:

4 in favor: @mkoeppe @kwankyu @kiwifb @NathanDunfield
3 against: @tobiasdiez @tornaria @orlitzky

Please vote!

pkgs/sage-conf/pyproject.toml.m4

saraedum · 2024-04-23T21:40:21Z

I vote -1 on this PR.

mkoeppe · 2024-04-27T23:03:51Z

I'll note that I am the developer who has been maintaining these distribution packages in pkgs/. I have built and maintain the templating facility (via m4) because synchronizing the version information and other metadata simplifies this maintenance work; it's a carefully thought out and executed part of my plan for the modularized distributions. I would ask reviewers here on the PR and similar PRs to take this into account.

mkoeppe · 2024-08-03T19:06:52Z

9 months later, this PR is still open.

The current tally:

4 in favor: @mkoeppe @kwankyu @kiwifb @NathanDunfield
4 against: @tobiasdiez @tornaria @orlitzky @saraedum

Please vote!

culler

This PR seems to replace a pyproject.toml file by an m4 template that can be used to
automatically update version information when generating the pyproject.toml file for a new version. I don't see anything particularly controversial about that. There are surely alternatives to using m4, but Sage is already using m4, so this choice does not make a significant difference.

…ls after sagemath#36885

orlitzky · 2026-02-06T03:27:45Z

pkgs/sage_conf no longer exists (c8a83f50c)

tobiasdiez added s: needs work and removed s: needs review labels Nov 2, 2023

kiwifb approved these changes Nov 2, 2023

View reviewed changes

kiwifb added s: positive review and removed s: needs review labels Nov 2, 2023

tobiasdiez removed the s: positive review label Nov 3, 2023

tobiasdiez added the s: needs work label Nov 3, 2023

tobiasdiez removed the s: positive review label Nov 4, 2023

vbraun force-pushed the develop branch 2 times, most recently from 883e05f to e349b00 Compare November 12, 2023 16:25

tobiasdiez mentioned this pull request Dec 6, 2023

Restructure sage.*.all for modularization, replace relative by absolute imports #36676

Closed

5 tasks

tornaria reviewed Apr 8, 2024

View reviewed changes

tornaria reviewed Apr 19, 2024

View reviewed changes

pkgs/sage-conf/pyproject.toml.m4 Show resolved Hide resolved

This comment was marked as off-topic.

Sign in to view

This comment was marked as abuse.

Sign in to view

culler approved these changes Aug 4, 2024

View reviewed changes

dimpase mentioned this pull request Aug 4, 2024

9 months later, this PR is still open. to record my vote: -1 #38469

Closed

tobiasdiez and others added 11 commits October 19, 2024 11:52

pkgs/sage-conf: Move version from setup.cfg to pyproject.toml

8efb067

remove version specifier from setup.cfg

0bf4423

pkgs/sage-conf: ini2toml setup.cfg > pyproject.toml

c80d5da

Remove symlinks to removed file pkgs/sage-conf/setup.cfg

bee86a8

build/pkgs/sage_conf/dependencies: Update

1d30884

build/pkgs/sage_conf/bootstrap: New

2749de6

pkgs/sage-conf/pyproject.toml.m4: Use m4 comment

5c6760e

.gitignore: Update

bec807a

pkgs/sage-conf/pyproject.toml: Remove (generated from ...m4)

72bbe3f

pkgs/sage-conf/pyproject.toml.m4: Change setuptools_wheel to setuptoo…

7e1e4a1

…ls after sagemath#36885

pkgs/sage-conf/pyproject.toml.m4: Allow Python 3.12

9977681

Uh oh!

Conversation

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

Uh oh!

📝 Checklist

⌛ Dependencies

Uh oh!

kiwifb commented Oct 29, 2023

Uh oh!

mkoeppe commented Oct 29, 2023

Uh oh!

kiwifb commented Oct 29, 2023

Uh oh!

mkoeppe commented Oct 29, 2023

Uh oh!

mkoeppe commented Oct 29, 2023

Uh oh!

tobiasdiez commented Oct 29, 2023

Uh oh!

mkoeppe commented Oct 29, 2023

Uh oh!

mkoeppe commented Nov 2, 2023

Uh oh!

tobiasdiez commented Nov 2, 2023

Uh oh!

mkoeppe commented Nov 2, 2023

Uh oh!

kiwifb left a comment

Choose a reason for hiding this comment

Uh oh!

mkoeppe commented Nov 2, 2023

Uh oh!

tobiasdiez commented Nov 3, 2023

Uh oh!

mkoeppe commented Nov 3, 2023

Uh oh!

tobiasdiez commented Nov 3, 2023

Uh oh!

mkoeppe commented Nov 3, 2023

Uh oh!

mkoeppe commented Nov 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tobiasdiez commented Nov 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkoeppe commented Dec 6, 2023

Uh oh!

tornaria commented Apr 8, 2024

Uh oh!

NathanDunfield commented Apr 8, 2024

Uh oh!

tornaria commented Apr 8, 2024

Uh oh!

tornaria Apr 8, 2024

Choose a reason for hiding this comment

Uh oh!

mkoeppe Apr 8, 2024

Choose a reason for hiding this comment

Uh oh!

orlitzky commented Apr 8, 2024

Uh oh!

mkoeppe commented Apr 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

orlitzky commented Apr 8, 2024

Uh oh!

mkoeppe commented Apr 8, 2024

Uh oh!

github-actions bot commented Apr 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkoeppe commented Apr 19, 2024

Uh oh!

Uh oh!

saraedum commented Apr 23, 2024

Uh oh!

mkoeppe commented Apr 27, 2024

Uh oh!

This comment was marked as off-topic.

mkoeppe commented Oct 28, 2023 •

edited

Loading

mkoeppe commented Nov 4, 2023 •

edited

Loading

tobiasdiez commented Nov 4, 2023 •

edited

Loading

mkoeppe commented Apr 8, 2024 •

edited

Loading

github-actions bot commented Apr 9, 2024 •

edited

Loading