Skip to content

Use pyproject and remove setup.py #188

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

edsu
Copy link
Contributor

@edsu edsu commented Jun 5, 2025

This commit includes changes to remove the use of setup.py and setuptools in favor of using the existing pyproject.toml and hatch.

A source layout was chosen, which required several changes such as:

  • pushing bagit.py into src/bagit/
  • pushing locale into src/bagit/locale

The gettext related code in setup.py was relocated to utils/locales.py It's very possible the way that the gettext functionality is setup needs some adjustment, as I'm new to using it.

Also we now rely on running tests with uv and pytest, which happens in the test github action. We could chose to update the PyPI publishing step to use uv build/publish too?

@edsu edsu force-pushed the rm-setuptools branch from 31f97b8 to 0d4a6ae Compare June 5, 2025 16:28
@@ -3,13 +3,13 @@ COMPILED_MESSAGES=$(patsubst %.po,%.mo, $(wildcard locale/*/LC_MESSAGES/bagit-py
all: messages compile

clean:
rm -f locale/*/LC_MESSAGES/*.mo
rm -f src/bagit/locale/*/LC_MESSAGES/*.mo

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 gettext files have been moved down into src.


[project]
name = "bagit"
dynamic = ["version"]
description = "Create and validate BagIt packages"
readme = {file = "README.rst", content-type = "text/x-rst"}
authors = [
{ name = "Ed Summers", email = "[email protected]" },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Andy wrote much of the original code, and Chris has added to it since I last touched it, so this seems right?

Copy link
Member

Choose a reason for hiding this comment

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

I guess that's an interesting question: do we try to list everyone who's made significant contributions in the past, or only the people who want to get emails when someone needs help?

Copy link
Contributor Author

@edsu edsu Jun 5, 2025

Choose a reason for hiding this comment

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

Yeah, good point -- maybe just you and me at this point? I guess things are busy at the fruit store for Mr Boyko...

Or perhaps just you, since I don't have rights to this repository anymore. I remain interested though!

@@ -27,14 +33,34 @@ Homepage = "https://libraryofcongress.github.io/bagit-python/"
[tool.ruff]
target-version = "py38"


Copy link
Contributor Author

Choose a reason for hiding this comment

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

hatch-vcs seems like it is doing this work now when i do a uv build?

@@ -27,14 +33,34 @@ Homepage = "https://libraryofcongress.github.io/bagit-python/"
[tool.ruff]
target-version = "py38"


[tool.setuptools_scm]

[tool.isort]
line_length = 110
default_section = "THIRDPARTY"
known_first_party = "bagit"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switching to using pytest to run the tests...

"src/bagit/locale/*.mo",
]


Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding these here means that ruff will ensure they are available when running tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that bagit.py is v1.0 compliant. At least there haven't been any complaints that it isn't?

Copy link
Member

Choose a reason for hiding this comment

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

I haven't heard any arguments otherwise. It's been a long time since I worked through that RFC and https://github.com/LibraryOfCongress/bagit-conformance-suite

@@ -1481,10 +1481,7 @@ def _make_parser():

checksum_args = parser.add_argument_group(
_("Checksum Algorithms"),
_(
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 was a formatting change that my local ruff suggested. I'm not sure why CI didn't care too...

This commit includes changes to remove the use of setup.py and
setuptools in favor of using the existing pyrproject.toml and hatch.

A source layout was chosen, which required several changes such as:

- pushing bagit.py into src/bagit/
- pushing locale into src/bagit/locale

The gettext related code in setup.py was relocated to utils/locales.py
It's possible the way that the gettext functionality is setup needs to
adjustment, as I'm new to using it.

Also we now rely on running tests with uv and pytest, which happens in the test
github action. We could chose to update the PyPI publishing step to use
uv build/publish too?
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