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

remove setup.py and migrate to a static pyproject.toml #388

Draft
wants to merge 7 commits into
base: trunk
Choose a base branch
from

Conversation

glyph
Copy link
Member

@glyph glyph commented Jun 19, 2024

Remove setup.py and migrate project metadata to a static pyproject.toml .

@glyph glyph marked this pull request as draft June 19, 2024 07:05
Comment on lines +6 to +11
# What follows is an exact copy of the runtime dependencies, because (A)
# incremental needs to be able to import `treq` to be able to import
# `treq.__version__` (see `tools.setuptools.dynamic` below), and (B) the
# `treq` module is set up such that everything is eagerly imported into the
# top-level namespace and thus sucks in every other dependency that we
# require.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is odd, because incremental exec()s the _version.py directly by my reading.

https://github.com/twisted/incremental/blob/5752c86d01e3fb188b65ef3a1a31427717184f6d/src/incremental/__init__.py#L379-L380

Comment on lines +81 to +82
[tool.setuptools.dynamic]
version = {attr = "treq.__version__"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see, you've tried to use setuptools attr and it's falling back to importing the module.

I don't see anything here equivalent to the use_incremental=True argument to setup(). It looks like we need to add support to incremental for direct integration with setuptools, like setuptools_scm has:

https://github.com/pypa/setuptools_scm/blob/102f56a896bc4ea8d4c58d9c49f455f15310de38/pyproject.toml#L76-L77

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup.

Copy link
Member Author

Choose a reason for hiding this comment

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

Filed twisted/incremental#90 for this so that we can fix it in incremental. Sad that this seems to be the main setuptools build extension entrypoint.

@twm
Copy link
Contributor

twm commented Sep 16, 2024

@glyph Do you have any plan to revive this now that Incremental supports pyproject.toml?

@glyph
Copy link
Member Author

glyph commented Sep 19, 2024

@glyph Do you have any plan to revive this now that Incremental supports pyproject.toml?

Thanks for the heads up, I would definitely like for this to be landed.

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.

2 participants