Sd2 1247 migrate pp from pip to poetry try upgrading packages #340
Conversation
WalkthroughThe modifications update the IBM API integration and project configuration. A new function, Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Code
participant IBMC as ibm_clients
participant TC as text_client
participant IAM as IAMAuthenticator
participant NLU as NaturalLanguageUnderstandingV1
Client->>IBMC: Request IBM API client
IBMC->>TC: Call text_client(api_key, version, service_url)
TC->>IAM: Initialize IAMAuthenticator with api_key
IAM-->>TC: Return IAM authenticator
TC->>NLU: Create NaturalLanguageUnderstandingV1 with authenticator & service_url
NLU-->>TC: Return configured NLU client
TC-->>IBMC: Return NLU client instance
IBMC-->>Client: Return IBM API client
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
pyproject.toml (2)
37-38: 🛠️ Refactor suggestionDynamic dependencies configuration will be ignored
The
[tool.setuptools.dynamic]section is trying to load dependencies from "requirements.txt", but you've migrated to Poetry for dependency management. This section is now redundant as Poetry will use the dependencies specified in the[tool.poetry.dependencies]section.-[tool.setuptools.dynamic] -dependencies = {file = ["requirements.txt"]}
14-16: 💡 Verification agent🧩 Analysis chain
Inconsistency between Python version requirement and classifiers
The
requires-pythonfield specifies ">=3.11", but the classifiers list Python 3.8, 3.9, and 3.10 compatibility. This inconsistency should be addressed.
🏁 Script executed:
#!/bin/bash # Check if there are any Python files that might not be compatible with Python 3.11 # Look for syntax or features that were deprecated or changed between Python 3.8-3.10 and 3.11 echo "Python version requirement in pyproject.toml: >=3.11" echo "Classifiers in pyproject.toml mention Python 3.8, 3.9, and 3.10" echo "Checking for potential compatibility issues..." # Check for uses of deprecated features in Python 3.11 echo "Searching for uses of deprecated features in Python 3.11..." rg -l "collections\.Mapping" || echo "No uses of collections.Mapping found (deprecated in 3.11)" rg -l "from distutils\." || echo "No uses of distutils found (deprecated in 3.11)" rg -l "asyncio\.coroutine" || echo "No uses of asyncio.coroutine found (deprecated in 3.11)" # Check for Python version imports or version checks echo "Checking for Python version-specific code..." rg "import sys" -A 2 -B 2 | rg "sys\.version" || echo "No explicit version checks found" echo "Consider updating the classifiers to match the requires-python field"Length of output: 1290
Update Python Classifiers in pyproject.toml to Reflect the Actual Version Requirement
- The
requires-pythonfield specifies ">=3.11", while the classifiers still list support for Python 3.8, 3.9, and 3.10.- The verification script confirmed that no deprecated features or explicit version checks are present, so the issue is solely with the classifier definitions.
- Please update the classifiers (lines 14-16 in pyproject.toml) to include supported versions starting from Python 3.11 (and above, as applicable) to ensure consistency.
🧹 Nitpick comments (3)
edenai_apis/apis/ibm/config.py (1)
3-3: Remove unused import
BearerTokenAuthenticatoris imported but not used anywhere in the code.-from ibm_cloud_sdk_core.authenticators import BearerTokenAuthenticator, IAMAuthenticator +from ibm_cloud_sdk_core.authenticators import IAMAuthenticator🧰 Tools
🪛 Ruff (0.8.2)
3-3:
ibm_cloud_sdk_core.authenticators.BearerTokenAuthenticatorimported but unusedRemove unused import:
ibm_cloud_sdk_core.authenticators.BearerTokenAuthenticator(F401)
pyproject.toml (2)
65-78: Duplicate project metadataThere's duplication between the
[project]and[tool.poetry]sections, which can lead to confusion and maintenance issues. Consider consolidating this information to avoid inconsistencies.You have two options:
- Keep only the
[tool.poetry]section and remove the[project]section if you're using Poetry exclusively- Keep both but ensure they're consistent and document the reason for having both
Since Poetry is now your build system, option 1 is recommended unless you have specific needs for the PEP 621 style metadata.
80-135: Comprehensive dependency specificationThe dependencies are now well specified with version constraints, which is good practice. However, there are a lot of dependencies, and some might not be necessary for all users.
Consider organizing dependencies into groups using Poetry's
[tool.poetry.group.<name>.dependencies]feature. For example:[tool.poetry.group.dev.dependencies] pylint = "^3.3.5" mypy = "^1.15.0" pytest = "^7.4.4" pytest-mock = "^3.14.0" pytest-cov = "^6.0.0" pytest-xdist = "^3.6.1" pytest-timeout = "^2.3.1" [tool.poetry.group.docs.dependencies] sphinx = "^8.2.3" furo = "^2024.8.6"This would allow users to install only what they need with commands like
poetry install --only mainorpoetry install --with dev.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
edenai_apis/apis/ibm/config.py(1 hunks)edenai_apis/apis/ibm/ibm_helpers.py(2 hunks)pyproject.toml(2 hunks)requirements.in(0 hunks)requirements.txt(0 hunks)
💤 Files with no reviewable changes (2)
- requirements.txt
- requirements.in
🧰 Additional context used
🪛 Ruff (0.8.2)
edenai_apis/apis/ibm/config.py
3-3: ibm_cloud_sdk_core.authenticators.BearerTokenAuthenticator imported but unused
Remove unused import: ibm_cloud_sdk_core.authenticators.BearerTokenAuthenticator
(F401)
🔇 Additional comments (5)
edenai_apis/apis/ibm/ibm_helpers.py (1)
1-1: Correctly updated IBM Watson import and exception handlingThe import statement and exception handling have been properly updated to use
ApiExceptionfromibm_watsoninstead of the previousWatsonApiExceptionfromwatson_developer_cloud.watson_service. This change aligns with the migration to newer IBM Watson APIs.Also applies to: 14-14
edenai_apis/apis/ibm/config.py (2)
7-15: Well-designed encapsulation of client creationThe new
text_clientfunction nicely encapsulates the creation of aNaturalLanguageUnderstandingV1client. This improves code organization and maintainability by isolating the client creation logic.
20-24: Updated client instantiation approachThe code properly updates the client instantiation to use the new
text_clientfunction, which leverages theIAMAuthenticatorfor authentication instead of the previousiam_apikeyapproach.pyproject.toml (2)
2-2: Update project metadataThe project name has been updated to "edenai-apis" and the Python version requirement has been increased to ">=3.11". The license file name has also been corrected to "LICENSE".
Also applies to: 6-7
137-139: Appropriate build system configurationThe build system configuration is correct for a Poetry project.
…ckages' of github.com:edenai/edenai-apis into SD2-1247-migrate-pp-from-pip-to-poetry-try-upgrading-packages
KyrianC
force-pushed
the
SD2-1247-migrate-pp-from-pip-to-poetry-try-upgrading-packages
branch
from
6eb4176 to
0f79194
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/test.yml (2)
1-1: File Naming Consistency:The header comment on line 1 indicates
# .github/workflows/app.yamlwhile the file is named.github/workflows/test.yml. Please update the header comment (or the filename) so that they consistently reflect the intended file name.
30-34: Install Poetry Step:The addition of the "Install Poetry" step using the official installation script is well implemented. As a best practice, consider verifying the integrity/authenticity of the script, if deemed necessary, and confirm that appending
"$HOME/.local/bin"to$GITHUB_PATHconsistently exposes Poetry on the runner's PATH.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
.github/workflows/test.yml(1 hunks)
🔇 Additional comments (3)
.github/workflows/test.yml (3)
28-29: Python Version Upgrade:The Python version has been updated to "3.12". Ensure that all project dependencies and tests are compatible with Python 3.12.
35-36: Clear Poetry Cache:Clearing the Poetry cache before installing dependencies is a solid approach to prevent issues from stale or corrupted cache data.
42-42: Switch to Poetry Dependency Installation:Replacing the pip-based dependency installation with
poetry install --no-interaction --no-rootaligns well with the PR’s objective to migrate to Poetry. Confirm that this change meets all dependency resolution requirements across environments.
KyrianC
force-pushed
the
SD2-1247-migrate-pp-from-pip-to-poetry-try-upgrading-packages
branch
from
41c1cfb to
57f5045
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
36-40: Debugging Configuration FilesThe "Debug" step outputs the contents of
pyproject.tomlandpoetry.lock, aiding in troubleshooting and verification of configuration changes. Ensure that this debugging output is acceptable (i.e., no sensitive information is exposed) and consider removing or conditioning it for production environments if not needed permanently.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (1)
.github/workflows/test.yml(1 hunks)
🔇 Additional comments (4)
.github/workflows/test.yml (4)
28-29: Python Version Upgrade to 3.12The workflow now uses Python 3.12. Please verify that all project dependencies, scripts, and tests are compatible with this version to avoid runtime issues.
30-34: Installation of PoetryA new step to install Poetry using the official installation script has been added. The configuration appends Poetry’s installation path to the system PATH, ensuring it’s available for subsequent steps. This change aligns with the PR objective of migrating dependency management to Poetry.
41-42: Clearing Poetry CacheClearing the Poetry cache (for the PyPI repository) is a useful step to prevent dependency conflicts during installation. Confirm that this step does not introduce significant delays in the CI process over repeated builds.
48-48: Switch to Poetry for Dependency InstallationReplacing the pip-based dependency installation with
poetry install --no-interaction --no-rootis consistent with the move to Poetry. Ensure that thepoetry.lockfile is up-to-date and version-controlled for reproducible builds.
KyrianC
force-pushed
the
SD2-1247-migrate-pp-from-pip-to-poetry-try-upgrading-packages
branch
from
c4052dd to
b3a0ec2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
47-47: Install Dependencies with Poetry
Changing the dependency installation command topoetry install --no-interaction --no-rootcorrectly leverages the Poetry-managed dependency tree. As an enhancement, consider caching the Poetry dependencies to further optimize build times in CI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (1)
.github/workflows/test.yml(1 hunks)
🔇 Additional comments (4)
.github/workflows/test.yml (4)
28-28: Update Python Version to 3.12
The workflow now uses Python 3.12. Please ensure that the entire codebase and all dependencies are compatible with this new version to prevent runtime issues.
30-34: Install Poetry Setup
The addition of the "Install Poetry" step is well implemented. Installing Poetry via the official installation script and appending its binary path to the system PATH aligns with our migration from pip to Poetry. Consider optionally verifying the Poetry version post-installation if you need to enforce a minimum version requirement.
36-39: Debug Step for pyproject.toml
Introducing a debug step to output the differences inpyproject.tomlis a useful troubleshooting aid. It helps quickly confirm that version bumps or dependency changes are being applied as expected.
40-41: Clear Poetry Cache
Clearing the Poetry cache for PyPI before dependency installation can prevent issues with stale packages. This step is appropriate given the shift to Poetry and will help maintain clean dependency installations.
to update version we can run:
poetry version+ one ofpatch,minor,major,prepatch,preminor,premajor,prereleaseto bump the version in pyproject.toml, then create git tagSummary by CodeRabbit