-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
Transition to pyproject.toml #187
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a significant restructuring of the project's configuration and dependency management. The project is transitioning from using Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Build as Build System
participant Metadata as importlib.metadata
participant Package as Hassil Package
Dev->>Build: Trigger package build
Build->>Metadata: Request package version
Metadata-->>Build: Return version (2.1.0)
Build->>Package: Configure package metadata
Package-->>Build: Package ready for distribution
This sequence diagram illustrates the new version retrieval and package build process, highlighting the simplified approach using Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
CHANGELOG.md
(1 hunks)hassil/VERSION
(0 hunks)hassil/_resources.py
(1 hunks)mypy.ini
(1 hunks)pyproject.toml
(1 hunks)requirements.txt
(0 hunks)requirements_dev.txt
(1 hunks)script/package
(1 hunks)setup.py
(0 hunks)tox.ini
(1 hunks)
💤 Files with no reviewable changes (3)
- setup.py
- hassil/VERSION
- requirements.txt
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🧰 Additional context used
📓 Path-based instructions (2)
script/package (6)
Pattern */**(html|markdown|md)
: - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern */**(html|markdown|md)
: - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern */**(html|markdown|md)
: - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern */**(html|markdown|md)
: - Use sentence-style capitalization also in headings.
Pattern */**(html|markdown|md)
: do not comment on HTML used for icons
Pattern */**(html|markdown|md)
: Avoid flagging inline HTML for embedding videos in future reviews for this repository.
hassil/_resources.py (6)
Pattern */**(html|markdown|md)
: - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern */**(html|markdown|md)
: - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern */**(html|markdown|md)
: - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern */**(html|markdown|md)
: - Use sentence-style capitalization also in headings.
Pattern */**(html|markdown|md)
: do not comment on HTML used for icons
Pattern */**(html|markdown|md)
: Avoid flagging inline HTML for embedding videos in future reviews for this repository.
🔇 Additional comments (8)
hassil/_resources.py (1)
3-5
: **Switching to importlib.metadata for version retrieval **
Using importlib.metadata.version("hassil")
is a modern and streamlined approach that eliminates the need for separate version files. However, verify that the package name "hassil" is consistent with how the project will be installed (e.g., if it's actually installed as pip install hassil
). Otherwise, importlib.metadata.PackageNotFoundError
may be raised.
Run the following script to confirm that the package name is correctly set and recognized by importlib.metadata
:
✅ Verification successful
Package name "hassil" is correctly configured and consistent
The package name "hassil" is properly configured in pyproject.toml
and used consistently in the version retrieval code. The package will be installed with this exact name, ensuring importlib.metadata.version("hassil")
will work correctly.
pyproject.toml
defines the package name asname = "hassil"
- The version retrieval in
_resources.py
uses the matching package name:importlib.metadata.version("hassil")
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# This script verifies that the "hassil" package is properly installed under that name
# and will be discoverable by importlib.metadata.
# 1. Search for lines referencing the name "hassil" in pyproject.toml or any other config
rg -A 3 'name\s*=\s*"hassil"'
# 2. Verify that "hassil" is spelled consistently across the codebase
rg -A 2 'importlib.metadata.version\("hassil"\)'
Length of output: 353
script/package (1)
12-12
: **Adopting the build module for packaging **
Transitioning from setup.py
to the modern build
module is a recommended best practice aligned with PEP 517. This approach is more standardized and ensures compatibility with pip-based workflows.
requirements_dev.txt (1)
Line range hint 1-8
: **Pinned development dependencies **
Pinning the versions for development tools helps ensure reproducible builds and consistent CI results. This is a good practice, especially when multiple contributors are involved or when gating merges with automated checks.
mypy.ini (1)
13-13
: **Adjusting type-checking for unicode_rbnf **
Allowing ignore_missing_imports
for unicode_rbnf.*
is acceptable if the library's type stubs are incomplete or missing. Review whether this setting can be tightened in the future to ensure type rigor.
pyproject.toml (4)
1-3
: LGTM! Build system configuration is appropriate.
The setuptools version and build backend are correctly specified.
28-31
: LGTM! Dependencies are well-defined.
The version constraints are appropriately specified using the recommended format, and the unicode-rbnf upgrade aligns with the PR objectives.
41-46
: LGTM! Package configuration is well-structured.
The package discovery settings and script entry point are correctly configured.
36-39
: Verify zip-safe compatibility
The zip-safe=true
setting indicates that this package can be run directly from a ZIP file. Please verify that:
- No file system operations depend on actual file paths
- No runtime resource loading that might break in a ZIP
✅ Verification successful
zip-safe=true
is compatible with this codebase
The codebase's file operations are safe for ZIP distribution:
- All file operations are for loading YAML configuration files at startup, not runtime
- Uses standard
open()
with UTF-8 encoding for YAML files - No absolute path manipulations or complex path resolution
- No runtime resource loading or file system dependencies
- Path imports are only used for type hints and file path parameters
The package can safely run from a ZIP file as it doesn't rely on actual file paths or perform runtime filesystem operations that would break in a ZIP context.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential file operations that might break zip-safe
rg -A 3 "open\(|__file__|os\.path|pathlib" hassil/
# Check for resource loading patterns
rg -A 3 "pkg_resources|importlib.resources|get_data" hassil/
Length of output: 1403
Script:
#!/bin/bash
# Check for any dynamic file/resource loading patterns
rg -A 3 "load|read_|from_file|resource" hassil/
# Check for any file path manipulations
rg -A 3 "dirname|abspath|realpath|resolve|absolute|relative" hassil/
# Look for any file-related imports
ast-grep --pattern 'import $_ from "fs"'
ast-grep --pattern 'from $_ import Path'
Length of output: 2404
@@ -1,6 +1,6 @@ | |||
[tox] | |||
env_list = | |||
py{38,39,310,311,312} | |||
py{38,39,310,311,312,313} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Update CI workflow to include Python 3.13 testing
The verification reveals:
- The project's dependencies (including unicode-rbnf) support Python 3.13
- The project requires Python >=3.8.0, which is compatible
- However, the CI workflow's Python matrix doesn't include 3.13 testing
To fully support Python 3.13:
- Update the Python versions matrix in
.github/workflows/ci.yaml
to include "3.13":
strategy:
matrix:
python_version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]
🔗 Analysis chain
Verify Python 3.13 compatibility testing
Adding Python 3.13 support is forward-thinking, but since it's still in development, we should ensure:
- CI environment has Python 3.13 available
- All dependencies support Python 3.13
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if any dependencies explicitly exclude Python 3.13
rg "Programming Language :: Python :: 3" $(fd -t f "pyproject.toml|setup.py|setup.cfg" -x echo {})
# Check if unicode-rbnf 2.2 supports Python 3.13
curl -s https://pypi.org/pypi/unicode-rbnf/2.2/json | jq -r '.info.classifiers[] | select(contains("Programming Language :: Python"))'
Length of output: 876
Script:
#!/bin/bash
# Check GitHub Actions workflow files for Python 3.13 setup
rg "python-version.*3\.13" .github/workflows/
# Check if there are any dependency specifications that might conflict with Python 3.13
rg "python_(version|requires|requires_dist)" $(fd -t f "pyproject.toml|setup.cfg" -x echo {})
# Check for any potential version-specific code or imports
rg "sys.version_info|@python_version|Python 3\.13" -g '!*.md' -g '!*.rst'
Length of output: 250
Script:
#!/bin/bash
# Check GitHub Actions workflow files for Python setup matrix
rg "python-version.*matrix" -A 5 .github/workflows/
# List all workflow files to ensure we don't miss any Python setup
fd workflow .github/workflows/
# Check for any Python version constraints in dependencies
rg "requires-python|python_requires" $(fd -t f "pyproject.toml|setup.cfg" -x echo {})
Length of output: 542
Script:
#!/bin/bash
# Check the matrix strategy definition in the CI workflow
rg "strategy:" -A 10 .github/workflows/ci.yaml
# Check the content of script/setup to see if it handles Python version compatibility
cat script/setup
Length of output: 1406
[project] | ||
name = "hassil" | ||
version = "2.1.0" | ||
license = {text = "Apache-2.0"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix license inconsistency
There's a mismatch between the license specifications:
- Line 8 specifies "Apache-2.0"
- Line 19 classifier indicates "MIT License"
Please update the classifier to match the Apache-2.0 license:
- "License :: OSI Approved :: MIT License",
+ "License :: OSI Approved :: Apache Software License",
Also applies to: 19-19
Summary by CodeRabbit
Release Notes for Version 2.1.0
New Features
unicode-rbnf
library to version 2.2Project Structure
pyproject.toml
for project configurationsetup.py
andrequirements.txt
Development
Compatibility