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

improve packaging, migrate to setup.cfg #397

Closed
wants to merge 3 commits into from

Conversation

deronnax
Copy link
Contributor

@deronnax deronnax commented Feb 19, 2024

declarative config (putting all the packaging information into the setup.cfg) is superior to imperative config (using code in setup.py) (source: setuptools themselve)

You lose the ability to do python setup.py test but managing tests with setup.py is deprecated and so is calling setup.py via the command line.

@deronnax deronnax marked this pull request as draft February 19, 2024 22:59
@deronnax
Copy link
Contributor Author

#398 should be merged first

@sergei-maertens
Copy link
Contributor

Shouldn't this entirely setup.py just be removed? As far as I can tell all the packing info is already in pyproject.toml and poetry-based. I found out about this the hard way in our fork, and there's a serious lack of documentation on how to actually build and publish the package:

poetry build
poetry publish

@Viicos
Copy link
Contributor

Viicos commented Feb 22, 2024

The relevant info is here:

[build-system]
requires = [
"poetry>=1.1.15",
"setuptools >= 40.1.0",
"wheel"
]
build-backend = "poetry.core.masonry.api"

the build backend points to Poetry so any setuptools related files (setup.py, setup.cfg) is currently/will be ignored.

If you want to stick with Poetry, I would suggest using:

[build-system]
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core.masonry.api"

There's no need to require poetry and all the dependencies when building

@deronnax
Copy link
Contributor Author

Ah damn. All this work for almost nothing! I wasn't sure Poetry was the way to go, because it's not used in CI, so I was mislead thinking Poetry was not used (and actually it might not be, since setup.py metadata are still relevant. But since the package is not built in the CI, we can't know).
Is the maintainer still out there? @pitbulk ?

@sergei-maertens
Copy link
Contributor

Yeah I don't know a lot about poetry, but it looks like the pip install -e . in the makefile does somehow use pyproject.toml and poetry build systems etc.

I've recently gone through the same surprises and frustrations - so some documentation improvements and cleanups (removing setup.py completely) would definitely clarify the situation.

@Viicos
Copy link
Contributor

Viicos commented Feb 22, 2024

I was mislead thinking Poetry was not used (and actually it might not be, since setup.py metadata are still relevant)

Build frontend tools such as pip will make use the build-system.build-backend key if present in pyproject.toml. So doing a plain pip install . locally will use poetry to install it (if the key is not present, build frontends should default to setuptools iirc).

However, the CI doesn't seem to handle packaging and publishing (as you mentioned), so maintainers might be forcing setuptools to be used in some way.

The metadata and packaging configuration would probably benefit from a large cleanup. We couldn't really do it in our fork to avoid merge conflicts

@deronnax
Copy link
Contributor Author

Agreed. I will do all these cleanup if the maintainer comes back

@pitbulk
Copy link
Contributor

pitbulk commented Feb 22, 2024

@deronnax, Maintainer here, what do you need?

@deronnax
Copy link
Contributor Author

You direction to do some clean up work. Do you use the setup.py or Poetry to manage dependencies and build a release?

@pitbulk
Copy link
Contributor

pitbulk commented Feb 22, 2024

To build the release I use setup.py

python setup.py sdist bdist_wheel upload -r pypi

or

python setup.py sdist bdist_wheel
twine upload dist/<package_name_and_version>-py3-none-any.whl

@sergei-maertens
Copy link
Contributor

But the setup.py is not picked up for local development due to the presence of pyproject.toml which specifies poetry as build tool.

In the current situation you need to make the same change to setup.py and pyproject.toml (e.g. when specifying new or newer dependencies, as that's how I found this out).

Please pick one method.

@pitbulk
Copy link
Contributor

pitbulk commented Feb 26, 2024

When I was porting from setup.py to pyproject.toml I experienced some challenges, that why I ended with both.

You can read this thread where people experienced similar issues:
https://discuss.python.org/t/user-experience-with-porting-off-setup-py/37502/18

@Viicos
Copy link
Contributor

Viicos commented Feb 26, 2024

I can give it a shot if you are ok with it?

@Viicos Viicos mentioned this pull request May 7, 2024
@deronnax
Copy link
Contributor Author

I'll let @Viicos do the work on his PR.

@deronnax deronnax closed this Jun 18, 2024
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.

4 participants