modernize packaging and add release automation #270

dhellmann · 2024-07-24T16:05:08Z

This PR updates the packaging files to use pyproject.toml instead of
setup.py and setup.cfg.

It also adds dynamic version computation using setuptools-scm, to derive
the version of the package being built from the tag, so that there is
only one source of truth for the version number.

Finally, it adds a github action that will trigger when a tag is pushed
to build and upload the sdist and wheel files for the new version. There
are some manual steps that need to be completed before that github action
will work, to give github permission to publish to pypi.org.

Related to #269

dhellmann · 2024-07-24T16:05:45Z

@martinpopel , this is related to #269

dhellmann · 2024-07-24T16:29:23Z

It looks like the linter doesn't like the way I replaced __version__ in __main__.py.

martinpopel · 2024-07-26T13:04:20Z

Great thanks.
I've run the CI and I see the ruff linter still does not like the from .version import __version__ line because F401 .version.__version__ imported but unused even if __version__ is now included in __all__.
I don't understand why. Or is CI confused by the last forced push?
Should we add # noqa: F401 to the import line?

dhellmann · 2024-07-26T13:09:38Z

Great thanks. I've run the CI and I see the ruff linter still does not like the from .version import __version__ line because F401 .version.__version__ imported but unused even if __version__ is now included in __all__. I don't understand why. Or is CI confused by the last forced push? Should we add # noqa: F401 to the import line?

I spotted a missing comma when I ran ruff locally, so I've pushed a new version. Let's see if CI likes that better.

martinpopel · 2024-07-26T13:27:55Z

This missing comma is fixed, but we have another problem. I admit I never really understood the imports in sacrebleu.
It is a question whether we still need the compat.py module for old style API access (<= 1.4.10).
However, even if we decided to remove it, I would prefer to release at least one version where these methods are issuing a deprecation warning.

dhellmann · 2024-07-26T14:06:49Z

I can fix the circular import problem by importing from the new location.

I don't see a tox environment for running the tests, how do you usually do that locally?

dhellmann · 2024-07-26T18:30:11Z

I worked out how to run the tests, and the latest draft passed a bunch of them locally before I pushed it up for validation in CI.

dhellmann · 2024-07-26T20:59:29Z

I missed some places where automation was using the old approach to building packages.

martinpopel · 2024-07-27T20:10:50Z

The build now fails with:

/tmp/build-env-uf1gjo1m/lib/python3.12/site-packages/setuptools_scm/git.py:167: UserWarning: "/home/runner/work/sacrebleu/sacrebleu" is shallow and may cause errors
  warnings.warn(f'"{wd.path}" is shallow and may cause errors')
error: Multiple top-level packages discovered in a flat-layout: ['data', 'sacrebleu'].

dhellmann · 2024-07-27T20:29:52Z

The build now fails with:

/tmp/build-env-uf1gjo1m/lib/python3.12/site-packages/setuptools_scm/git.py:167: UserWarning: "/home/runner/work/sacrebleu/sacrebleu" is shallow and may cause errors
  warnings.warn(f'"{wd.path}" is shallow and may cause errors')
error: Multiple top-level packages discovered in a flat-layout: ['data', 'sacrebleu'].

I've been trying to keep this change as small as possible, but there seem to be a few non-standard things about the layout and packaging of the source tree. I could go all in and fix all that, if you're interested? I don't want to overstep, though.

Use setuptools to package in the same way, but use pyproject.toml for compatibility with tools relying on PEP 517.

Use setuptools-scm to get a version from the tag when released instead of looking for the contents of a file. Generate a version.py file to replace the static value in __init__.py.

This action responds when a tag is pushed to the main repository to build and upload both sdist and wheels to pypi.org. More details in https://help.github.com/en/actions/language-and-framework-guides/using-python-with-github-actions#publishing-to-package-registries

dhellmann · 2024-07-27T20:38:06Z

Let's see if specifying the finder parameters is enough.

martinpopel · 2024-07-29T10:05:10Z

I like this, thanks. @mjpost do you agree with merging (as it is mostly you who publishes to PyPI)?

dhellmann · 2024-07-29T12:39:57Z

sacrebleu/__init__.py

I have my editor configured to auto-format using ruff, and that has introduced a lot more change in these files than is truly needed.

dhellmann · 2024-07-29T12:40:12Z

sacrebleu/__init__.py

+    "sentence_chrf",
+    "corpus_ter",
+    "sentence_ter",
+    "__version__",


Adding __version__ here was needed.

dhellmann · 2024-07-29T12:40:25Z

sacrebleu/__init__.py

+    get_source_file,
+    smart_open,
+)
+from .version import __version__


This import replaces the old static definition.

dhellmann · 2024-07-29T12:40:57Z

sacrebleu/metrics/base.py


-from .. import __version__
+from ..version import __version__


This import change eliminates a circular dependency between the sacrebleu package and this module.

mjpost · 2024-07-29T20:18:20Z

Thanks, @dhellmann! This looks good to me. Don't worry about overstepping, we appreciate the contribution.

I assume I have to upload a secret somewhere for pypi authentication. It wasn't obvious to me what to do, however. Can you comment?

mjpost · 2024-07-29T20:20:15Z

Hmm, I found the link to the docs that you provided. I'll go through the OpenID authorization.

mjpost · 2024-07-29T20:23:22Z

The build now fails with:
/tmp/build-env-uf1gjo1m/lib/python3.12/site-packages/setuptools_scm/git.py:167: UserWarning: "/home/runner/work/sacrebleu/sacrebleu" is shallow and may cause errors
  warnings.warn(f'"{wd.path}" is shallow and may cause errors')
error: Multiple top-level packages discovered in a flat-layout: ['data', 'sacrebleu'].
I've been trying to keep this change as small as possible, but there seem to be a few non-standard things about the layout and packaging of the source tree. I could go all in and fix all that, if you're interested? I don't want to overstep, though.

Should I merge the current PR? Or did you want to go ahead and fix this as well? We could also consider this as a second PR, if you are interested.

dhellmann · 2024-07-29T20:47:45Z

The build now fails with:
/tmp/build-env-uf1gjo1m/lib/python3.12/site-packages/setuptools_scm/git.py:167: UserWarning: "/home/runner/work/sacrebleu/sacrebleu" is shallow and may cause errors
  warnings.warn(f'"{wd.path}" is shallow and may cause errors')
error: Multiple top-level packages discovered in a flat-layout: ['data', 'sacrebleu'].
I've been trying to keep this change as small as possible, but there seem to be a few non-standard things about the layout and packaging of the source tree. I could go all in and fix all that, if you're interested? I don't want to overstep, though.
Should I merge the current PR? Or did you want to go ahead and fix this as well? We could also consider this as a second PR, if you are interested.

Let's leave it as a second PR. I have some deadlines at work that may mean it'll take a while before I can get to it, but if you file an issue and assign it to me I'll come back to it.

mjpost · 2024-07-29T21:04:27Z

Great, thanks, Doug!

martinpopel · 2024-07-30T11:42:22Z

Within this PR, the CI tests work without any errors, but strangely after merging to master, there is an error:

Run python -m build
/opt/hostedtoolcache/Python/3.[11](https://github.com/mjpost/sacrebleu/actions/runs/10151812795/job/28071892972#step:6:12).9/x64/bin/python: No module named build.__main__; 'build' is a package and cannot be directly executed
Error: Process completed with exit code 1.

Any ideas how to fix it?

dhellmann · 2024-07-30T12:48:31Z

This line in the log output looks suspicious

WARNING: sacrebleu 0.1.dev1+g5af5a3a does not provide the extra 'build'

It's probably coming from this line https://github.com/mjpost/sacrebleu/pull/270/files#diff-87c8be2ac3b248aef84cc474af838d7cc461b7f8fb7f5444ac75f4d8fc02e377R32

I used "[build]" there instead of "[dev]" so I think it's not installing the "build" package at all.

add a tox environment for testing package builds

9f7dfcf

dhellmann force-pushed the 269-add-github-action-for-publishing branch from c98db25 to f2152ac Compare July 24, 2024 17:02

dhellmann force-pushed the 269-add-github-action-for-publishing branch from f2152ac to 715f024 Compare July 26, 2024 13:09

dhellmann force-pushed the 269-add-github-action-for-publishing branch from 715f024 to cf527f5 Compare July 26, 2024 18:27

dhellmann force-pushed the 269-add-github-action-for-publishing branch from cf527f5 to 54737c3 Compare July 26, 2024 20:58

dhellmann added 3 commits July 27, 2024 16:37

replace setup.py with static pyproject.toml

e50dc4b

Use setuptools to package in the same way, but use pyproject.toml for compatibility with tools relying on PEP 517.

dynamically generate version from tags

dd20ed1

Use setuptools-scm to get a version from the tag when released instead of looking for the contents of a file. Generate a version.py file to replace the static value in __init__.py.

dhellmann force-pushed the 269-add-github-action-for-publishing branch from 54737c3 to 6bc3b26 Compare July 27, 2024 20:37

martinpopel approved these changes Jul 29, 2024

View reviewed changes

dhellmann commented Jul 29, 2024

View reviewed changes

mjpost merged commit 5af5a3a into mjpost:master Jul 29, 2024
19 checks passed

mjpost mentioned this pull request Jul 29, 2024

Further standardize the layout and packaging #271

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

modernize packaging and add release automation #270

modernize packaging and add release automation #270

dhellmann commented Jul 24, 2024

dhellmann commented Jul 24, 2024

dhellmann commented Jul 24, 2024

martinpopel commented Jul 26, 2024

dhellmann commented Jul 26, 2024

martinpopel commented Jul 26, 2024

dhellmann commented Jul 26, 2024

dhellmann commented Jul 26, 2024

dhellmann commented Jul 26, 2024

martinpopel commented Jul 27, 2024

dhellmann commented Jul 27, 2024

dhellmann commented Jul 27, 2024

martinpopel commented Jul 29, 2024

dhellmann Jul 29, 2024

dhellmann Jul 29, 2024

dhellmann Jul 29, 2024

dhellmann Jul 29, 2024

mjpost commented Jul 29, 2024

mjpost commented Jul 29, 2024

mjpost commented Jul 29, 2024

dhellmann commented Jul 29, 2024

mjpost commented Jul 29, 2024

martinpopel commented Jul 30, 2024

dhellmann commented Jul 30, 2024

modernize packaging and add release automation #270

modernize packaging and add release automation #270

Conversation

dhellmann commented Jul 24, 2024

dhellmann commented Jul 24, 2024

dhellmann commented Jul 24, 2024

martinpopel commented Jul 26, 2024

dhellmann commented Jul 26, 2024

martinpopel commented Jul 26, 2024

dhellmann commented Jul 26, 2024

dhellmann commented Jul 26, 2024

dhellmann commented Jul 26, 2024

martinpopel commented Jul 27, 2024

dhellmann commented Jul 27, 2024

dhellmann commented Jul 27, 2024

martinpopel commented Jul 29, 2024

dhellmann Jul 29, 2024

Choose a reason for hiding this comment

dhellmann Jul 29, 2024

Choose a reason for hiding this comment

dhellmann Jul 29, 2024

Choose a reason for hiding this comment

dhellmann Jul 29, 2024

Choose a reason for hiding this comment

mjpost commented Jul 29, 2024

mjpost commented Jul 29, 2024

mjpost commented Jul 29, 2024

dhellmann commented Jul 29, 2024

mjpost commented Jul 29, 2024

martinpopel commented Jul 30, 2024

dhellmann commented Jul 30, 2024