-
Notifications
You must be signed in to change notification settings - Fork 4
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
move from setuptools setup.py -> project.toml #58
Conversation
WalkthroughThe changes update the project’s CI workflows and packaging configuration. The GitHub workflows now install dependencies with "python -m pip", introduce "poetry" for building, and install the resulting wheel from the dist folder. The README now reflects building via pyproject.toml, and a new pyproject.toml file provides project metadata and dependency definitions. Additionally, the recipe has been updated to use poetry-core instead of setuptools, and the legacy setup.py has been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Runner
participant Pip as PIP Installer
participant Poetry as Poetry Builder
participant Dist as Distribution Folder
Runner->>Pip: Install flake8 & pytest (using python -m pip)
Runner->>Pip: Install poetry (using python -m pip)
Runner->>Poetry: Build package (poetry build)
Poetry->>Dist: Generate wheel file
Runner->>Pip: Install built wheel from dist/*.whl
Poem
🪧 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: 1
🧹 Nitpick comments (3)
recipe/meta.yaml (1)
27-28
: Clarify inclusion of setuptools in run requirements.
While the host now uses "poetry-core", "setuptools" still appears in the run requirements. Confirm that setuptools is necessary during runtime, or consider removing it if no longer needed..github/workflows/python-publish.yml (1)
22-27
: Adopt Poetry-based build commands.
The updated "Install dependencies" step installs flake8 and pytest using "python -m pip install", installs Poetry, and then runs "poetry build" followed by installing the generated wheel. This approach aligns with the overall migration to Poetry. For improved reproducibility, consider pinning the Poetry version explicitly.README.md (1)
29-29
: Update Section Title to Reflect New Build ProcessThe heading "Install with setup.py:" is now misleading because the project has migrated from a setuptools‑based setup to the new pyproject.toml/poetry build approach. Consider updating the section title to something like "Build from Source" or "Installation from Source" to better reflect the current build process.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/python-app.yml
(1 hunks).github/workflows/python-package.yml
(1 hunks).github/workflows/python-publish.yml
(1 hunks)README.md
(1 hunks)pyproject.toml
(1 hunks)recipe/meta.yaml
(1 hunks)setup.py
(0 hunks)
💤 Files with no reviewable changes (1)
- setup.py
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
34-34: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (10)
pyproject.toml (6)
1-4
: Review backend configuration in build-system.
The build backend is currently set to "setuptools.build_meta" with "setuptools>=45" and "wheel". However, note that in recipe/meta.yaml the host requirements have been updated to require "poetry-core >=1.2.0". Please verify that this is intentional. If the goal is to adopt a Poetry-based build workflow, consider updating the backend to "poetry.core.masonry.api" to maintain consistency.
5-28
: Project metadata configuration looks solid.
The [project] section clearly defines the name, version, description, authors, Python version requirement, keywords, license information, and dependencies. Everything appears well organized. Just double-check that including certain packages (e.g. "setuptools") in dependencies is intentional and necessary at runtime.
29-31
: Project URLs defined correctly.
The Homepage URL is appropriately specified in the [project.urls] section.
32-34
: CLI entry point setup is appropriate.
Mapping the command "pridepy" to "pridepy:main" in the [project.scripts] section is correctly configured for the command-line interface.
35-38
: Setuptools configuration is in place.
The [tool.setuptools] block, which specifies the packages and Python modules, is correctly set up for package building.
39-41
: Package data configuration is clear.
The package data patterns declared in [tool.setuptools.package-data] ensure that ancillary files (like *.txt and *.md) are included.recipe/meta.yaml (1)
20-23
: Update host requirements with Poetry-core.
Replacing "setuptools" with "poetry-core >=1.2.0" in the host requirements is a positive step towards adopting a Poetry-based build process. Please ensure that this change aligns with your overall build configuration, especially in light of the build backend specified in pyproject.toml..github/workflows/python-publish.yml (1)
28-29
: Review potential redundancy in build steps.
The workflow first builds the package using "poetry build" and then issues a separate "python -m build" command. This may result in duplicate builds or conflicting artifacts. Please verify if both steps are necessary or if they can be consolidated..github/workflows/python-package.yml (1)
20-25
: Consistent dependency installation with Poetry.
The installation step now leverages "python -m pip install" for flake8 and pytest, installs Poetry, executes "poetry build", and then installs the resulting wheel file. This updated process is consistent with the new packaging workflow..github/workflows/python-app.yml (1)
28-32
: Streamlined installation and build process.
The "Install dependencies" step installs necessary tools (flake8, pytest, Poetry), builds the package using "poetry build", and installs the built wheel from the "dist" folder. This sequence aligns with the transitions in your workflows. Just ensure that the built artifacts are correctly updated before the subsequent linting and testing steps run.
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: 0
🧹 Nitpick comments (2)
pyproject.toml (2)
74-83
: Code Formatting Configuration (Black): Minor Version Consideration.
The Black configuration is clearly specified with a custom line length and target version. Note that the target-version is set to "py39" while the project dependency specifies "^3.7". Consider aligning these if you intend to support earlier Python versions or confirm that 3.9-specific formatting is acceptable.
93-106
: Static Type Checking (Mypy): Verify Python Version Alignment.
The Mypy configuration is thorough; however, note that the "python_version" is set to "3.9" even though the project claims support for Python "^3.7". Make sure this reflects your actual deployment target or adjust accordingly to avoid potential discrepancies during static analysis.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (11)
pyproject.toml (11)
1-12
: Metadata Configuration: Comprehensive and Properly Structured.
The metadata block defines essential fields such as project name, version, description, authors, maintainers, readme, and license. Ensure that the "maintainers" field aligns with PEP 621 (if applicable) or your internal standards.
13-21
: Project Keywords: Clear and Domain-Relevant.
The list of keywords effectively captures the project's context in proteomics and bioinformatics.
22-34
: Classifiers: Informative and Well-Selected.
These classifiers correctly indicate the intended audience, license type, operating system independence, and the supported Python versions.
35-37
: Package Inclusion: Correct Configuration.
The package section accurately includes the "pridepy" module.
38-44
: Static Files Inclusion: Adequate.
Including key assets such as LICENSE, README.md, and text/markdown files ensures they are bundled in the distribution.
46-55
: Runtime Dependencies: Properly Defined.
The dependencies block fixes a minimum Python version (^3.7) and constrains key libraries (like requests, ratelimit, etc.) to specific versions. Double-check these constraints to ensure compatibility across your environments.
57-62
: Development Dependencies: Well Specified.
All essential tools for testing, formatting, and static analysis (pytest, black, isort, pytest-cov, mypy) are clearly declared in the dev dependency group.
64-69
: Project URLs: Informative and Accurate.
The URLs provide useful links (PyPI, documentation, repository, etc.) that support both users and contributors.
71-73
: Script Entry Point: Defined Correctly.
The entry point maps the "pridepy" command to the main function in the pridepy module, ensuring that the CLI behaves as expected.
84-92
: Import Sorting (isort) Configuration: Well Aligned with Black.
The isort settings complement the Black configuration, ensuring consistent formatting of import statements throughout the project.
107-109
: Build System: Correctly Configured.
Specifying "poetry-core>=1.2.0" and using "poetry.core.masonry.api" as the build-backend adheres to modern packaging best practices.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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: 0
🔭 Outside diff range comments (1)
README.md (1)
29-36
: Update section header for clarity.The heading "Install with setup.py:" is now misleading since the project has transitioned to using Poetry via pyproject.toml for building. To avoid confusion, please update the header to something like "Build from Source Using Poetry" (or a similar title) that reflects the new packaging approach.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
32-32: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
33-33: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
34-34: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
35-35: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
🧹 Nitpick comments (1)
README.md (1)
34-34
: Maintain markdown command style consistency.The command line showing "$ poetry build" includes a shell prompt, which might trigger markdownlint (MD014) warnings about dollar signs without output. Consider either removing the dollar sign or providing sample output if applicable, to better conform with markdown style guidelines.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
34-34: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
34-34: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
Summary by CodeRabbit