Skip to content
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

Pin ndx-pose<0.2.0 #1978

Merged
merged 2 commits into from
Oct 1, 2024
Merged

Pin ndx-pose<0.2.0 #1978

merged 2 commits into from
Oct 1, 2024

Conversation

talmo
Copy link
Collaborator

@talmo talmo commented Oct 1, 2024

Description

Pin ndx-pose<0.2.0 until we fix compatibility with breaking API changes.

Types of changes

  • Bugfix
  • New feature
  • Refactor / Code style update (no logical changes)
  • Build / CI changes
  • Documentation Update
  • Other (explain)

Does this address any currently open issues?

[list open issues here]

Outside contributors checklist

  • Review the guidelines for contributing to this repository
  • Read and sign the CLA and add yourself to the authors list
  • Make sure you are making a pull request against the develop branch (not main). Also you should start your branch off develop
  • Add tests that prove your fix is effective or that your feature works
  • Add necessary documentation (if appropriate)

Thank you for contributing to SLEAP!

❤️

Summary by CodeRabbit

  • New Features

    • Updated version constraints for the ndx-pose package across various configuration files to ensure compatibility with versions below 0.2.0.
    • Specified version for tensorflow to ==2.7.0 for improved stability.
    • Introduced platform-specific requirements for tensorflow-macos and tensorflow-metal.
  • Bug Fixes

    • Adjusted indentation for better readability and consistency in dependency management.
  • Chores

    • Cleaned up comments and unnecessary constraints in configuration files to streamline package management.

Copy link

coderabbitai bot commented Oct 1, 2024

Walkthrough

The changes primarily focus on updating version constraints for the ndx-pose package across multiple configuration files, ensuring compatibility with versions below 0.2.0. Additional modifications include specifying the version of the tensorflow package and adjusting the formatting of dependencies in various environment files. These updates enhance dependency management for the sleap package and its associated environments.

Changes

Files Change Summary
.conda/meta.yaml, .conda_mac/meta.yaml, environment_mac.yml, environment_no_cuda.yml Updated ndx-pose version constraint to <0.2.0 in respective files.
environment.yml Changed ndx-pose constraint to <0.2.0, updated tensorflow to ==2.7.0, modified cudnn version specifier.
pypi_requirements.txt Updated ndx-pose to <0.2.0, refined tensorflow constraints, added urllib3 and updated protobuf.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Environment
    participant PackageManager

    User->>Environment: Request to set up environment
    Environment->>PackageManager: Check dependencies
    PackageManager->>PackageManager: Validate version constraints
    PackageManager-->>Environment: Confirm valid setup
    Environment-->>User: Environment ready
Loading

🐇 In the meadow, hops a little hare,
With version constraints, we show we care.
ndx-pose now snug, under two point oh,
In every file, the changes flow.
Dependencies tidy, like a neat little nest,
Our package is ready, we’ll pass every test! 🌼


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between cd955b2 and 5d15a52.

📒 Files selected for processing (1)
  • .conda_mac/meta.yaml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .conda_mac/meta.yaml

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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 and nitpick comments (8)
environment_mac.yml (1)

42-42: Approved: Version constraint for 'ndx-pose' correctly implemented.

The addition of - conda-forge::ndx-pose <0.2.0 aligns with the PR objectives to pin the 'ndx-pose' package to a version lower than 0.2.0. This change will prevent potential compatibility issues with breaking API changes in newer versions.

For improved clarity, consider adding a comment explaining the reason for this version constraint. For example:

# Pin ndx-pose to <0.2.0 due to compatibility issues with breaking API changes in newer versions
- conda-forge::ndx-pose <0.2.0

This will help future maintainers understand the rationale behind this specific version constraint.

environment_no_cuda.yml (1)

44-44: LGTM! Consider adding a comment explaining the version constraint.

The addition of the version constraint for ndx-pose is correct and aligns with the PR objectives. This change effectively pins the package to versions lower than 0.2.0, which should help avoid compatibility issues with breaking API changes.

Consider adding a brief comment explaining why this version constraint is necessary. This will help future maintainers understand the reasoning behind this specific version pinning. For example:

# Pin ndx-pose to <0.2.0 due to compatibility issues with breaking API changes
- conda-forge::ndx-pose <0.2.0
pypi_requirements.txt (2)

Line range hint 52-55: LGTM: Additional dependency constraints added for compatibility.

The added constraints for urllib3 and protobuf are good additions to ensure compatibility with other dependencies and to make the GUI work in Windows.

Consider adding these constraints to the project's documentation or a separate DEPENDENCIES.md file to explain why these specific versions are required. This will help future maintainers understand the reasoning behind these constraints.

🧰 Tools
🪛 LanguageTool

[grammar] ~41-~41: Did you mean the proper noun “Apple Silicon”?
Context: ...ested since we do not offer a wheel for apple silicon atm. tensorflow-macos==2.9.2; sys_platf...

(APPLE_PRODUCTS)


Line range hint 1-55: Overall, the changes look good and address the PR objective.

The main goal of pinning ndx-pose<0.2.0 has been achieved, and additional dependency constraints have been added to improve compatibility and functionality across different platforms. These changes should help maintain a stable development environment.

To further improve dependency management:

  1. Consider using a tool like pip-compile to generate and maintain a requirements.lock file. This would ensure reproducible builds across different environments.
  2. Regularly review and update dependencies to stay current with security patches and new features.
  3. Set up automated dependency checking (e.g., with Dependabot) to be notified of new versions and potential security vulnerabilities.
🧰 Tools
🪛 LanguageTool

[grammar] ~41-~41: Did you mean the proper noun “Apple Silicon”?
Context: ...ested since we do not offer a wheel for apple silicon atm. tensorflow-macos==2.9.2; sys_platf...

(APPLE_PRODUCTS)

environment.yml (2)

43-43: Approved: Version constraint for ndx-pose added as intended.

The addition of - conda-forge::ndx-pose <0.2.0 aligns with the PR objective to pin the ndx-pose package to a version lower than 0.2.0. This change will help prevent potential compatibility issues with newer versions.

Consider adding a comment explaining why this version constraint is necessary. This will help future maintainers understand the reasoning behind this decision.


40-40: Approved: TensorFlow version pinned with a note for future update.

The specification of sleap/label/dev::tensorflow ==2.7.0 with the TODO comment is a good temporary solution. It ensures a specific version is used while acknowledging the need for a future update.

To ensure the TODO is not forgotten, would you like me to create a GitHub issue to track the task of updating TensorFlow to the main label when it's ready?

.conda_mac/meta.yaml (1)

60-60: Fix formatting and approve version constraint.

The version constraint for ndx-pose has been correctly added as per the PR objective. However, there's a minor formatting issue:

Please apply this change to fix the formatting:

-    - conda-forge:: <0.2.0
+    - conda-forge::ndx-pose <0.2.0
.conda/meta.yaml (1)

58-58: Summary: ndx-pose version constraint successfully implemented

The changes in this file successfully implement the version constraint for ndx-pose (<0.2.0) in both the host and run environments. This addresses the PR objective of pinning the ndx-pose package version until compatibility issues with breaking API changes can be resolved.

The modifications are minimal and focused, maintaining consistency across the package lifecycle without affecting other dependencies. This approach should effectively prevent potential issues arising from incompatible ndx-pose versions.

Consider documenting this temporary version constraint in a prominent location (e.g., README or CHANGELOG) to ensure it's not overlooked in future updates. Also, create an issue to track the resolution of the compatibility problems with ndx-pose 0.2.0+, ensuring this constraint can be removed once those issues are addressed.

Also applies to: 93-93

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between ef803f6 and cd955b2.

📒 Files selected for processing (6)
  • .conda/meta.yaml (2 hunks)
  • .conda_mac/meta.yaml (2 hunks)
  • environment.yml (1 hunks)
  • environment_mac.yml (1 hunks)
  • environment_no_cuda.yml (1 hunks)
  • pypi_requirements.txt (1 hunks)
🔇 Additional comments (5)
pypi_requirements.txt (1)

40-40: LGTM: Version constraint for ndx-pose added as intended.

This change aligns with the PR objective to pin ndx-pose to a version lower than 0.2.0, which should prevent compatibility issues with breaking API changes in newer versions.

To ensure this change doesn't introduce any unintended consequences, please run the following script to check for any usage of ndx-pose features that might be affected by this version constraint:

✅ Verification successful

Verification Successful: Usage of ndx-pose Confirmed.

The ndx-pose dependency is used in the following files:

  • sleap/io/dataset.py
  • sleap/io/format/ndx_pose.py

The version constraint ndx-pose<0.2.0 ensures compatibility with the current usage, preventing potential issues from newer versions.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for usage of ndx-pose features that might be affected by the version constraint

# Search for imports and usage of ndx-pose
rg --type python -e "from ndx_pose" -e "import ndx_pose" -e "ndx_pose\." -C 5

Length of output: 110


Script:

#!/bin/bash
# Description: Check for usage of ndx-pose features that might be affected by the version constraint

# Search for imports and usage of ndx-pose using file globbing for Python files
rg -g "*.py" -e "from ndx_pose" -e "import ndx_pose" -e "ndx_pose\." -C 5

Length of output: 3875

.conda_mac/meta.yaml (2)

92-92: LGTM: Version constraint correctly added.

The version constraint for ndx-pose has been correctly added in the requirements.run section, consistent with the PR objective and the change in the requirements.host section.


60-60: Summary: Consistent version pinning for ndx-pose

The changes in both requirements.host and requirements.run sections consistently pin the ndx-pose package to versions below 0.2.0. This ensures that the version constraint is applied during both build and runtime, which aligns well with the PR objective.

These changes effectively address the compatibility issues mentioned in the PR description until they can be resolved.

Also applies to: 92-92

.conda/meta.yaml (2)

93-93: LGTM: ndx-pose version constraint added to run environment

The addition of conda-forge::ndx-pose <0.2.0 to the run requirements is consistent with the change in the host environment. This ensures that the version constraint is applied both during build and runtime.

Let's verify if this constraint is applied consistently across all relevant configuration files:

#!/bin/bash
# Description: Check for consistency of ndx-pose constraint across configuration files

# Test: Search for ndx-pose specifications in all yaml files
rg --type yaml 'ndx-pose' 

# Test: Search for ndx-pose specifications in all requirements files
rg 'ndx-pose' *requirements*.txt

# Test: Search for ndx-pose specifications in setup.py
rg 'ndx-pose' setup.py

58-58: LGTM: ndx-pose version constraint added to host environment

The addition of conda-forge::ndx-pose <0.2.0 to the host requirements aligns with the PR objective to pin the ndx-pose package version. This constraint ensures compatibility during the build process.

Let's verify if this constraint is consistent with other dependencies:

✅ Verification successful

Verified: ndx-pose version constraint is consistently applied across all dependencies

The constraint conda-forge::ndx-pose <0.2.0 is uniformly specified in all relevant environment and requirements files, ensuring compatibility and preventing conflicts.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any conflicts with the new ndx-pose constraint

# Test: Search for any other ndx-pose specifications
rg --type yaml 'ndx-pose' .conda/meta.yaml

# Test: Check if there are any other dependencies that might be affected by this change
rg --type yaml 'pose' .conda/meta.yaml

Length of output: 457

Copy link

codecov bot commented Oct 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.48%. Comparing base (7ed1229) to head (5d15a52).
Report is 54 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1978      +/-   ##
===========================================
+ Coverage    73.30%   75.48%   +2.17%     
===========================================
  Files          134      133       -1     
  Lines        24087    24625     +538     
===========================================
+ Hits         17658    18587     +929     
+ Misses        6429     6038     -391     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@roomrys roomrys merged commit 10aae76 into develop Oct 1, 2024
15 checks passed
@roomrys roomrys deleted the talmo/pin-ndx-pose branch October 1, 2024 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants