Update Saccharis #51314
Update Saccharis #51314
Conversation
… sha256 hash, and added --single-version-externally-managed --record=record.txt args to setup.py install as per bioconda test suite
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve modifications to the Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![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 and nitpick comments (1)
recipes/saccharis/meta.yaml (1)
11-13: Consider using a more flexible URL formatThe change to use a specific version in the URL is good for reproducibility. However, it may make future updates more challenging. Consider using a templated URL like the original, but with the correct file name format:
url: https://github.com/saccharis/SACCHARIS_2/releases/download/v{{ version }}/saccharis-{{ version }}_1.tar.gzThis would maintain flexibility while still pointing to the correct file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/saccharis/meta.yaml (3 hunks)
🧰 Additional context used
🔇 Additional comments (5)
recipes/saccharis/meta.yaml (5)
2-3: LGTM: Version and hash updatesThe version has been updated to a development release (2.0.1.dev21), and the hash value has been correspondingly updated. These changes are consistent with updating the package source.
28-28: LGTM: Updated entry pointsThe entry points for
show_family_categoriesandupdate_dbhave been updated to reflect changes in the package's internal structure or naming conventions. These changes are consistent with the package update.Also applies to: 31-31
76-82: Excellent: Improved test commandsThe test commands have been updated to use the
--helpflag. This is a great improvement as it:
- Prevents file write permission issues during testing.
- Ensures that the commands are available and can display their help information.
- Speeds up the testing process by not executing the full command functionality.
This change aligns well with best practices for Conda recipe testing.
87-88: LGTM: Improved description formattingThe package description has been split into two lines, improving readability without changing the content. This is a minor but welcome improvement to the recipe's formatting.
Line range hint
1-88: Request: Clarification on bugfixThe PR objectives mention a bugfix for installation issues, but there's no explicit mention of this in the recipe changes. Could you please provide more information about the specific installation issues that were addressed? This would help in understanding how the changes in this recipe relate to the bugfix.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
recipes/saccharis/meta.yaml (2)
11-12: Consider using a more flexible URL format.While the new URL is more specific, which is good for reproducibility, it hardcodes the version number. This might make future updates more error-prone. Consider using a format that still uses the
{{ version }}variable:url: https://github.com/saccharis/SACCHARIS_2/releases/download/v{{ version }}/saccharis-{{ version }}_2.tar.gzThis way, you only need to update the version number at the top of the file for future releases.
35-37: Approve build requirements, but fix indentation.Adding build requirements for Python and setuptools is good practice and ensures the correct build environment. However, the indentation seems to be incorrect.
Please adjust the indentation to 4 spaces to match the rest of the file:
build: - python - setuptools
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/saccharis/meta.yaml (3 hunks)
🧰 Additional context used
🔇 Additional comments (6)
recipes/saccharis/meta.yaml (6)
1-3: LGTM: Version and hash updates are consistent with the PR objective.The version update to "2.0.1.dev21" aligns with the PR's goal of adding a bugfix. The hash value has been appropriately updated to match the new source file.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
17-17: Clarify the build number increment.The build number has been increased from 0 to 2, which is correct when making changes to the package without changing the upstream version. However, jumping from 0 to 2 might indicate a missed intermediate build. Could you please clarify if there was an intermediate build or if this is intentional?
27-27: LGTM: Entry point updates appear consistent.The changes to the entry points for
saccharis.show_family_categoriesandsaccharis.update_dbseem to reflect updates in the package's internal structure or naming conventions. These changes appear consistent with the package's naming scheme.To ensure these changes are correct, please verify that these new entry points exist in the package code and function as expected.
Also applies to: 30-30
77-84: LGTM: Test command updates improve testing robustness.The addition of
--helpto these test commands is a good practice. It prevents file write permission issues during testing and aligns with the comment about piping test output to null. These changes will make the package testing more robust and less likely to fail due to environment-specific issues.
89-90: LGTM: Description formatting improves readability.Splitting the description into two lines improves readability without altering the content. The line break is placed at a logical point in the sentence, making it easier to read in the YAML format.
Line range hint
1-90: Overall, the changes look good with some minor suggestions for improvement.The updates to the SACCHARIS package recipe generally align with the PR objective of adding a bugfix. The version update, hash change, build number increment, and test command modifications all contribute to improving the package's functionality and testing robustness.
Here's a summary of the main points and recommendations:
- The version and hash updates are appropriate.
- Consider using a more flexible URL format that still includes the
{{ version }}variable for easier future updates.- Clarify the reason for jumping from build number 0 to 2.
- The entry point updates appear consistent, but verify they match the package code.
- Fix the indentation for the newly added build requirements.
- The test command updates and description formatting changes are improvements.
After addressing these minor points, the recipe should be in good shape for merging.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
|
@BiocondaBot please add label |
|
Thanks @AlexSCFraser |
Add bugfix for installation issues.
Please read the guidelines for Bioconda recipes before opening a pull request (PR).
General instructions
@BiocondaBot please add labelcommand.@bioconda/corein a comment.Instructions for avoiding API, ABI, and CLI breakage issues
Conda is able to record and lock (a.k.a. pin) dependency versions used at build time of other recipes.
This way, one can avoid that expectations of a downstream recipe with regards to API, ABI, or CLI are violated by later changes in the recipe.
If not already present in the meta.yaml, make sure to specify
run_exports(see here for the rationale and comprehensive explanation).Add a
run_exportssection like this:with
...being one of:{{ pin_subpackage("myrecipe", max_pin="x") }}{{ pin_subpackage("myrecipe", max_pin="x.x") }}{{ pin_subpackage("myrecipe", max_pin="x.x") }}(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin="x.x.x") }}(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin=None) }}while replacing
"myrecipe"with eithernameif aname|lowervariable is defined in your recipe or with the lowercase name of the package in quotes.Bot commands for PR management
Please use the following BiocondaBot commands:
Everyone has access to the following BiocondaBot commands, which can be given in a comment:
@BiocondaBot please update@BiocondaBot please add labelplease review & mergelabel.@BiocondaBot please fetch artifactsYou can use this to test packages locally.
Note that the
@BiocondaBot please mergecommand is now depreciated. Please just squash and merge instead.Also, the bot watches for comments from non-members that include
@bioconda/<team>and will automatically re-post them to notify the addressed<team>.