Conversation
- Add pyproject.toml and uv.lock for uv-based dependency management. - Update Makefile to use 'uv build' for package building. - Update .github/workflows/ci.yml to use astral-sh/setup-uv and uv commands. - Update .circleci/config.yml to install and use uv for testing. - Add uv installation instructions to README.rst and README_en.rst.
📝 WalkthroughWalkthroughThe PR removes pypy2 from CI, introduces uv-based build/install commands across CI and Makefile, adds a pyproject.toml build configuration, comments out twine in dev requirements, and updates docs/README to recommend Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
README.rst (1)
36-40: Consider usinguv pip installfor README context.For end-users simply installing the package from a README,
uv pip install pypinyinis more appropriate thanuv add pypinyin. Theuv addcommand is primarily for managing project dependencies inpyproject.toml, whileuv pip installis for direct environment installation—the typical use case for package installation instructions.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.circleci/config.yml.github/workflows/ci.ymlMakefileREADME.rstREADME_en.rstdocs/index.rstpyproject.tomlrequirements_dev.txt
💤 Files with no reviewable changes (1)
- .circleci/config.yml
🔇 Additional comments (5)
pyproject.toml (1)
74-80: LGTM: Setuptools configuration looks correct.The package configuration properly specifies all packages, enables package data inclusion, and disables zip-safe mode. The dynamic version sourcing from
pypinyin.__version__is a clean approach.README_en.rst (1)
36-40: LGTM: UV installation instructions added clearly.The UV installation documentation is well-formatted and consistent with the existing pip installation section. This provides users with an alternative installation method.
Makefile (1)
38-39: Same verification needed for publish_test target.As with the
publishtarget, ensure thatuv buildproduces artifacts compatible withtwine upload --repository test..github/workflows/ci.yml (2)
35-35: LGTM!Changing from
py.testtopython -m pytestis a good practice. It's more explicit and ensures the correct Python interpreter is used.
41-42: Useuv buildfor modernized Python package building.The project's
pyproject.tomlcorrectly defines the build-system withsetuptools.build_metabackend, which is compatible withuv build. The tool will produce both source distributions (.tar.gz) and wheels in thedist/directory, making the file patterndist/*.tar.gzcorrect for installing from the source distribution.
| uv pip install --system -r requirements_dev.txt | ||
| uv pip install --system . |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
uv pip install --system flag GitHub Actions CI best practices
💡 Result:
Summary best practices for using uv pip install --system in GitHub Actions CI
-
Prefer ephemeral virtual environments, not --system
- Default uv behavior (create a .venv) is safer and reproducible. Use uv venv or let uv create per-job venvs. [1][2]
-
Use --system only in CI/container contexts and with caution
- The docs say --system is “appropriate in continuous integration and containerized environments” because it mutates the system Python. Use it only when you intentionally want system-wide installs inside a disposable runner/container. [1][2]
-
Avoid PEP 668 / “externally managed” failures
- Many distros/uv setups mark system Python as externally managed; uv will refuse to modify it and pip may require break-system-packages. If you hit “externally managed” errors, either install into a venv, use distro packages (apt), or use an image where system Python is writable. [4][5][8]
-
Prefer installing CLI tools with uv tool install
- If you need globally available executables (ruff, pre-commit, etc.), prefer uv tool install or uvx rather than pip installing packages into /usr. That avoids system-package mutation. [1][2][3]
-
If you must use --system, ensure runner permissions and target interpreter
- On GitHub-hosted runners you usually cannot sudo; either use a container with writable Python or point uv to a writable interpreter with --python=$(which python). Test for permission errors (dist-packages write failures). [3][6]
-
Reproducibility: pin Python and packages
- In CI, pin the Python version (uv --python 3.x or use a runner image) and lock dependencies (requirements.txt/constraints or uv lock) so system installs are repeatable. [1]
-
Troubleshooting checklist
- If uv pip install --system fails:
a) Check for “externally managed” / PEP 668 message → use venv or distro package. [5][8]
b) Check permissions → use container or writable interpreter. [3]
c) Try uv pip install --python=$(which python) instead of --system. [7]
d) For CLIs, prefer uv tool install. [1]
- If uv pip install --system fails:
Selected sources
- uv docs (Using environments / --system explanation). [1]
- Stack Overflow discussion: using --system in CI and warning. [2]
- GitHub issues showing permission / behavior problems and workarounds. [3][5][7]
- Additional community guidance on CI usage and caveats. [6][8]
If you want, I can draft a small GitHub Actions job example showing (a) recommended pattern with uv venv, and (b) a container-based job using uv pip install --system safely.
🌐 Web query:
uv documentation recommended installation method for CI environments
💡 Result:
Recommended: install the standalone uv binary (curl https://astral.sh/uv/install.sh | sh) and use uvx for CI/one‑off automation. uvx is the recommended mode for CI/CD; alternatively uv's --system option can be used to target the system Python but is meant for CI with caution. [1][2][3]
Sources:
- uv installation + installers. [1]
- uv docs: uvx recommended for CI / one‑off executions (example in package docs). [2]
- uv CLI docs: --system option intended for CI (warning about modifying system Python). [3]
Consider using uvx instead of uv pip install --system in CI.
The --system flag mutates the system Python and is not the recommended CI approach. The uv documentation recommends uvx as the preferred method for CI/one-off executions. Additionally, --system installations may fail on systems with PEP 668 "externally managed" Python (increasingly common in Linux distributions), where uv will refuse to modify the system Python.
If system-wide installation is necessary, ensure the runner environment allows modifications or use a container with a writable Python installation. Otherwise, use virtual environments (uv's default behavior) or uvx for better CI portability.
🤖 Prompt for AI Agents
.github/workflows/ci.yml around lines 30-31: the workflow uses "uv pip install
--system -r requirements_dev.txt" and "uv pip install --system ." which mutates
the system Python and can fail under PEP 668 or locked runners; replace these
with CI-friendly invocations using uvx (e.g., use "uvx pip install -r
requirements_dev.txt" and "uvx pip install .") so uv runs the commands in a
transient, non-system context, or alternatively remove --system to rely on uv's
virtualenv behavior or run the job in a container/runner that permits system
installs if you truly need system-wide installation.
| * License: MIT license | ||
| * PyPI: https://pypi.org/project/pypinyin | ||
| * Python version: 2.7, pypy, pypy3, 3.4, 3.5, 3.6, 3.7, 3.8, 3.9 | ||
| * Python version: 2.7, pypy3, 3.4, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10, 3.11, 3.12, 3.13 |
There was a problem hiding this comment.
Verify consistency with pyproject.toml Python version requirements.
The documentation lists specific Python versions (2.7, pypy3, 3.4-3.13), but pyproject.toml line 10 specifies requires-python = ">=2.6, !=3.0.*, !=3.1.*, !=3.2.*, <4", which technically allows Python 2.6 and 3.3. Ensure the documentation accurately reflects the actual supported versions, or update pyproject.toml to match the documented versions.
🤖 Prompt for AI Agents
In docs/index.rst around line 20, the listed Python versions (2.7, pypy3,
3.4–3.13) are inconsistent with pyproject.toml's requires-python (>=2.6,
!=3.0.*, !=3.1.*, !=3.2.*, <4) which allows 2.6 and 3.3; update either docs or
pyproject.toml to match: either remove 2.6 and 3.3 from allowed versions in
pyproject.toml to match the docs, or add 2.6 and 3.3 to docs (and mention pypy
support) so both files state the same supported versions, and run a quick
test/build to ensure packaging/CI accept the chosen requires-python spec.
| [build-system] | ||
| requires = ["setuptools>=42", "wheel"] | ||
| build-backend = "setuptools.build_meta" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
setuptools 42 Python version support requirements
💡 Result:
Setuptools 42.0.0 (released 23 Nov 2019) added explicit support for Python 3.8 (fixed 3.8 Windows ABI handling) and continued to support Python 2.7 (Python‑2 support was removed later in setuptools 45). Source: setuptools changelog v42.0.0. [1]
References
- setuptools changelog — v42.0.0 (23 Nov 2019). [1]
🌐 Web query:
setuptools 42 Python 2.6 support compatibility
💡 Result:
No — setuptools 42 is not compatible with Python 2.6. Setuptools dropped Python 2.6 support in v37.0.0 (Nov 20, 2017); later, setuptools v45 (Jan 11, 2020) dropped all Python 2 support (so Python 2.7 only kept on the 44.x branch). Use setuptools <37 if you must run on Python 2.6. [1][2]
Sources:
- setuptools release history (v37.0.0 drop of Python 2.6; v45 drop of Python 2). [1]
- Setuptools Python 2 sunset documentation. [2]
Setuptools 42 does not support Python 2.6; either update supported Python versions or lower the setuptools requirement.
Setuptools dropped Python 2.6 support in v37.0.0 (November 2017). The setuptools>=42 requirement in the build system is incompatible with Python 2.6. If line 10 declares Python 2.6 support, either:
- Remove Python 2.6 from the supported versions, or
- Lower the setuptools requirement to
setuptools<37to maintain Python 2.6 compatibility.
(Setuptools 42 does support Python 2.7, so this constraint only affects Python 2.6.)
🤖 Prompt for AI Agents
pyproject.toml lines 1-3: the build-system requires "setuptools>=42" which is
incompatible with Python 2.6 if your package declares Python 2.6 support (per
comment at line 10); either remove Python 2.6 from the supported Python versions
metadata (so you can keep setuptools>=42), or relax the build-system requirement
to a setuptools version that still supports 2.6 (e.g., setuptools<37) to retain
2.6 compatibility—update the pyproject.toml accordingly to reflect the chosen
option.
| description = "汉字拼音转换模块/工具." | ||
| readme = "README.rst" | ||
| dynamic = ["version"] | ||
| requires-python = ">=2.6, !=3.0.*, !=3.1.*, !=3.2.*, <4" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
uv Python 2.6 2.7 support minimum version requirement
💡 Result:
uv requires Python 3.8 or newer (requires-python >=3.8). It does not support Python 2.6 or 2.7. [1][2]
Sources:
- PyPI project metadata (Requires: Python >=3.8). [1]
- uv documentation on Python versions (installation/supported versions). [2]
🏁 Script executed:
# Check if there are CI/test configurations that specify tested Python versions
fd -t f "(\.github|\.gitlab|tox|\.travis)" -x cat {} \; 2>/dev/null | head -100Repository: mozillazg/python-pinyin
Length of output: 1007
🏁 Script executed:
# Look for any Python 2.6-specific code or comments in the codebase
rg "python.*2\.6|2\.6.*python" -i --max-count=20Repository: mozillazg/python-pinyin
Length of output: 5668
🏁 Script executed:
# Check pyproject.toml and setup files for more context
git ls-files | grep -E "(pyproject|setup|tox)" | head -20Repository: mozillazg/python-pinyin
Length of output: 108
Update requires-python to reflect actual support and incompatibility with uv.
The project declares Python 2.6+ support in pyproject.toml, but this is outdated. The CHANGELOG explicitly states that Python 2.6 and 3.3 are no longer tested and compatibility is not guaranteed. Additionally, the project uses uv (present in uv.lock), which requires Python 3.8+, making Python 2.6 incompatible with the project's tooling. Update requires-python to match the minimum Python version the project actually supports and tests.
🤖 Prompt for AI Agents
In pyproject.toml around line 10, the requires-python entry currently allows
Python 2.6 and other old versions which is incorrect given the CHANGELOG and the
project's dependency on uv (which requires Python >=3.8); update the
requires-python spec to reflect the actual minimum supported Python version
(e.g., ">=3.8, <4") so tooling and package metadata accurately prevent
installation on unsupported Python versions.
| release = [ | ||
| "bumpversion", | ||
| "twine", | ||
| "wheel>=0.21" | ||
| ] |
There was a problem hiding this comment.
Inconsistency: twine included in release group but commented out in requirements_dev.txt.
The release dependency group includes twine, but requirements_dev.txt comments out twine on line 12. This creates an inconsistency. If twine is needed (as indicated by the Makefile), it should be uncommented in requirements_dev.txt or the release workflow should be updated.
🤖 Prompt for AI Agents
In pyproject.toml around lines 68-72 the release group includes "twine", but
requirements_dev.txt has twine commented out (line 12), causing an
inconsistency; either uncomment the twine entry in requirements_dev.txt so the
dev requirements match the release group (and ensure Makefile targets relying on
twine work), or remove twine from the release group / update the Makefile to not
require it; pick the option that matches project intent (recommended: uncomment
twine in requirements_dev.txt to align with the release workflow).
| * License: MIT license | ||
| * PyPI: https://pypi.org/project/pypinyin | ||
| * Python version: 2.7, pypy, pypy3, 3.4, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10, 3.11, 3.12, 3.13 | ||
| * Python version: 2.7, pypy3, 3.4, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10, 3.11, 3.12, 3.13 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for Python 2.7 references in CI and test configurations
rg -n "python.*2\.7|py27" -g "*.{yml,yaml,ini,cfg,toml}" -C2Repository: mozillazg/python-pinyin
Length of output: 641
🏁 Script executed:
# Also check for the CI workflow file mentioned
fd -t f "ci\.yml|\.github.*ci" | head -20Repository: mozillazg/python-pinyin
Length of output: 49
🏁 Script executed:
# Check the setup.py or pyproject.toml for python_requires
fd -t f "(setup\.py|pyproject\.toml|setup\.cfg)" | xargs ls -laRepository: mozillazg/python-pinyin
Length of output: 247
🏁 Script executed:
cat -n setup.py | head -50Repository: mozillazg/python-pinyin
Length of output: 1648
🏁 Script executed:
cat -n tox.iniRepository: mozillazg/python-pinyin
Length of output: 1315
🏁 Script executed:
# Check if these files were modified in the current PR
git diff HEAD~1 HEAD -- tox.ini setup.py pyproject.toml | head -100Repository: mozillazg/python-pinyin
Length of output: 95
🏁 Script executed:
# Look for GitHub Actions workflow files
fd -t f -e yml -e yaml ".github" | head -20Repository: mozillazg/python-pinyin
Length of output: 49
🏁 Script executed:
# Check what Python versions are tested in GitHub Actions
find .github -name "*.yml" -o -name "*.yaml" 2>/dev/null | xargs catRepository: mozillazg/python-pinyin
Length of output: 1611
🏁 Script executed:
# Check README.rst to see the full context around line 15
cat -n README.rst | sed -n '1,30p'Repository: mozillazg/python-pinyin
Length of output: 873
Remove unsupported Python versions from the compatibility list.
The README lists Python 2.7, 3.4, 3.5, and 3.6, but the GitHub Actions CI workflow only tests Python 3.7+, 3.8, 3.9, 3.10, 3.11, 3.12, 3.13, pypy3.9, and pypy3.10. Remove versions that are not actively tested.
🤖 Prompt for AI Agents
In README.rst around line 15, the Python compatibility list includes unsupported
versions; update the line to remove Python 2.7, 3.4, 3.5, and 3.6 and replace
the list with the actively tested interpreters (Python 3.7, 3.8, 3.9, 3.10,
3.11, 3.12, 3.13, pypy3.9, pypy3.10) so the README matches the GitHub Actions CI
matrix.
| Sphinx>=3.0.4 | ||
| tox | ||
| twine | ||
| #twine |
There was a problem hiding this comment.
Critical: Twine is still required by the Makefile.
Commenting out twine will break the publish and publish_test targets in the Makefile, which still invoke twine upload on lines 33 and 39. Either restore the twine dependency or update the Makefile to use uv publish (if UV supports publishing).
🔎 Proposed fix: restore twine or migrate to UV publish
Option 1: Restore twine dependency
-#twine
+twineOption 2: Update Makefile to use UV publish (if supported)
Check if UV supports publishing and update the Makefile accordingly. If UV doesn't support direct publishing, twine must remain as a dependency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #twine | |
| twine |
🤖 Prompt for AI Agents
In requirements_dev.txt around line 12 the twine dependency was commented out
but the Makefile still calls `twine upload` on lines 33 and 39; either restore
`twine` to requirements_dev.txt so those targets work, or update the Makefile
publish and publish_test targets to use `uv publish` (only if UV actually
supports publishing) and remove/replace all `twine` invocations; after making
the change run the Makefile publish targets locally or in CI to verify the
chosen approach works and update CI/dev docs accordingly.
1 participant
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.