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

feat: migrate from setup.py/setuptools to pyproject.toml/hatch #1163

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

regisb
Copy link
Contributor

@regisb regisb commented Nov 22, 2024

While investigating issue #956, I realized that the issue might be resolved if we switched from setuptools to hatch as package builder. But to achieve that, we also need to migrate from setup.py to pyproject.toml. This was a long overdue chore, as setup.py is no longer recommended (see PEP 621).

I'm not sure whether we should be merging this change now, later (or ever). I think this should be safe... So I would lean towards merging it now (and eventually migrate plugins and cookiecutter to pyproject.toml as well). But I'm curious what ya'll think @kdmccormick @DawoudSheraz.

This is an attempt both to modernize the packaging and to fix mypy
testing in editable mode (issue #956).

Packaging seems to work, but mypy testing still fails. Will now attempt
to migrate from setuptools to hatch for building.
The official Python packaging guide recommends hatch over setuptools.
Beyond that official recommendation, getting rid of setuptools allows us
to resolve issue #956 on mypy-checking of plugins in editable mode.

Close #956.
pip install .
pip install -r requirements/dev.txt

bootstrap-dev-plugins: bootstrap-dev ## Install dev requirements and all supported plugins
Copy link
Contributor

@DawoudSheraz DawoudSheraz Nov 25, 2024

Choose a reason for hiding this comment

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

Any particular reasons for merging this? I get this is being made part of full, just thinking if someone was using this in their automation and can break.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The make are not supposed to be part of the Tutor API, so I think it's OK to make breaking changes here. The goal of this change is to get rid of an unused command.

Comment on lines +12 to +14
pip-compile ${COMPILE_OPTS} requirements/base.in
pip-compile ${COMPILE_OPTS} requirements/dev.in
pip-compile ${COMPILE_OPTS} requirements/docs.in
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for having COMPILE_OPTS? What values can it have?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a technique that I also used in edx-platform to de-duplicate the list of *.in files in the Makefile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

2 participants