Add r-saccharis #51468
Add r-saccharis #51468
Conversation
… sha256 hash, and added --single-version-externally-managed --record=record.txt args to setup.py install as per bioconda test suite
# Conflicts: # recipes/saccharis/meta.yaml
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several key changes to the Possibly related PRs
Suggested labels
Suggested reviewers
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: 1
🧹 Outside diff range and nitpick comments (5)
recipes/r-saccharis/conda_build_config.yaml (1)
5-5: Add a newline character at the end of the file.To adhere to YAML best practices and prevent potential issues with certain tools, please add a newline character at the end of the file.
Apply this change to add a newline at the end of the file:
r_base: - 3.6.3 +🧰 Tools
🪛 yamllint
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
recipes/r-saccharis/meta.yaml (4)
10-13: LGTM: Source information is well-defined.The source URL and SHA256 hash are correctly specified using Jinja2 variables. Using GitHub releases for source distribution is a good practice.
Consider adding a comment above the
filehashvariable (line 4) to indicate that it needs to be updated when the package version changes. This can help prevent oversight during future updates.
24-50: Requirements are well-defined, but consider version constraints.The host and run requirements are appropriately specified and include necessary dependencies for a phylogenetic tree package.
Consider adding version constraints to critical dependencies to ensure compatibility. For example:
requirements: host: - r-base >=3.5.0 - r-ggplot2 >=3.3.0 - bioconductor-ggtree >=2.0.0 run: - r-base >=3.5.0 - r-ggplot2 >=3.3.0 - bioconductor-ggtree >=2.0.0This helps maintain consistency across different environments and prevents potential issues with incompatible package versions.
52-54: Test command is present, but could be more comprehensive.The current test command checks if the package can be loaded, which is a good basic test.
Consider adding more comprehensive tests to verify key functionalities of the package. For example:
test: commands: - 'Rscript -e "library(rsaccharis)"' - 'Rscript -e "if (!requireNamespace(\"rsaccharis\", quietly = TRUE)) stop(\"rsaccharis not available\")"' - 'Rscript -e "if (!exists(\"A_load_data\", where = asNamespace(\"rsaccharis\"))) stop(\"A_load_data function not found\")"' - 'Rscript -e "if (!exists(\"B_plots_all\", where = asNamespace(\"rsaccharis\"))) stop(\"B_plots_all function not found\")"'These additional tests check for the availability of the package and the existence of key functions mentioned in the package description.
56-63: About section is informative and well-structured.The about section provides comprehensive information about the package, including its purpose, functionality, and usage instructions.
Consider adding a
doc_urlfield pointing to the package's documentation, if available. For example:about: home: 'https://github.com/saccharis/rsaccharis' doc_url: 'https://saccharis.github.io/rsaccharis/' # Add this line if documentation is available license: 'GPL 3' # ... rest of the section remains the sameThis would provide users with easy access to the package's documentation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- recipes/r-saccharis/build.sh (1 hunks)
- recipes/r-saccharis/conda_build_config.yaml (1 hunks)
- recipes/r-saccharis/meta.yaml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- recipes/r-saccharis/build.sh
🧰 Additional context used
🪛 yamllint
recipes/r-saccharis/conda_build_config.yaml
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
recipes/r-saccharis/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (3)
recipes/r-saccharis/conda_build_config.yaml (1)
1-5: Verify if the specified package versions are the latest compatible ones.The file specifies numpy 1.17 and R 3.6.3, which are quite old versions. While pinning versions is good for reproducibility, using outdated versions might lead to missing out on performance improvements, bug fixes, or compatibility with newer packages.
Consider checking if newer versions are compatible with r-saccharis and other dependencies in the Bioconda ecosystem. This could potentially improve the package's performance and maintainability.
Would you like me to help check for the latest compatible versions of numpy and R base?
To verify the latest versions and their compatibility, we can run the following script:
This script will help us understand if there are newer versions available and if r-saccharis has any specific version constraints that require these older versions.
🧰 Tools
🪛 yamllint
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
recipes/r-saccharis/meta.yaml (2)
1-8: LGTM: Package metadata is well-defined.The package metadata section is correctly structured using Jinja2 variables. The version, name, and build number are appropriately set, and the file hash is provided for integrity checking.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
1-63: Overall, the r-saccharis recipe is well-structured and follows Bioconda guidelines.The recipe provides all necessary information for building and distributing the r-saccharis package. The suggested improvements, if implemented, will enhance the recipe's robustness and user-friendliness. These include:
- Adding a comment for updating the file hash.
- Fixing the
run_exportssyntax.- Considering version constraints for dependencies.
- Adding more comprehensive test commands.
- Including a
doc_urlin the about section.Great job on creating this recipe! Once these minor adjustments are made, it should be ready for inclusion in the Bioconda repository.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
recipes/r-saccharis/meta.yaml
Outdated
| build: | ||
| noarch: generic | ||
| number: {{ build_number }} | ||
| rpaths: | ||
| - lib/R/lib/ | ||
| - lib/ | ||
| run_exports: | ||
| - { { pin_subpackage("r-saccharis", max_pin="x.x") } } |
There was a problem hiding this comment.
Build configuration looks good, but needs a minor fix.
The build configuration is well-structured with appropriate settings for an R package. However, there's a syntax issue in the run_exports section.
Fix the spacing in the run_exports section. Apply this diff:
- - { { pin_subpackage("r-saccharis", max_pin="x.x") } }
+ - {{ pin_subpackage("r-saccharis", max_pin="x.x") }}This removes the extra spaces within the Jinja2 brackets, which would cause a syntax error during package building.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| build: | |
| noarch: generic | |
| number: {{ build_number }} | |
| rpaths: | |
| - lib/R/lib/ | |
| - lib/ | |
| run_exports: | |
| - { { pin_subpackage("r-saccharis", max_pin="x.x") } } | |
| build: | |
| noarch: generic | |
| number: {{ build_number }} | |
| rpaths: | |
| - lib/R/lib/ | |
| - lib/ | |
| run_exports: | |
| - {{ pin_subpackage("r-saccharis", max_pin="x.x") }} |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
recipes/r-saccharis/meta.yaml (4)
10-13: LGTM: Source information is correctly specified.The source URL and SHA256 hash are properly defined. Good use of Jinja2 templating for version and hash.
Consider adding a comment above the
filehashvariable (line 4) to indicate its purpose, e.g.:# SHA256 hash of the source tarball {% set filehash = "F02EC69008F688440D7D4230751D1A8C9501F24F2382F399317591C4005DF19F"%}This improves readability and maintainability of the recipe.
24-50: LGTM: Dependencies are well-defined, but consider version constraints.The host and run requirements are correctly specified and include all necessary R packages and Bioconductor dependencies.
Consider adding version constraints to critical dependencies to ensure compatibility. For example:
requirements: host: - r-base >=3.5.0 - r-ggplot2 >=3.3.0 - bioconductor-ggtree >=2.0.0 run: - r-base >=3.5.0 - r-ggplot2 >=3.3.0 - bioconductor-ggtree >=2.0.0This helps maintain consistency across different environments and prevents potential issues with incompatible package versions.
52-54: LGTM: Basic test is present, but could be expanded.The current test ensures that the package can be loaded, which is a good basic check.
Consider expanding the tests to cover more functionality:
Add a test to check if key functions are available:
- 'Rscript -e "library(rsaccharis); stopifnot(exists(\"A_load_data\"), exists(\"B_plots_all\"))"'If possible, add a small dataset and test a core function:
- 'Rscript -e "library(rsaccharis); data(example_data); result <- A_load_data(example_data); stopifnot(!is.null(result))"'These additional tests would provide more confidence in the package's functionality.
56-63: LGTM: Metadata is well-defined and informative.The about section provides comprehensive information about the package, including its purpose, usage instructions, and licensing.
Consider adding a
dev_urlfield pointing to the GitHub repository for easier access to development resources:about: home: 'https://github.com/saccharis/rsaccharis' dev_url: 'https://github.com/saccharis/rsaccharis' license: 'GPL-3.0-or-later' license_family: GPL # ... (rest of the existing fields)Also, update the
licensefield to use SPDX identifierGPL-3.0-or-laterand addlicense_family: GPLfor better standardization.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/r-saccharis/conda_build_config.yaml (1 hunks)
- recipes/r-saccharis/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/r-saccharis/conda_build_config.yaml
[error] 2-2: no new line character at the end of file
(new-line-at-end-of-file)
recipes/r-saccharis/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (5)
recipes/r-saccharis/conda_build_config.yaml (2)
1-2: LGTM: r_base version specified correctly.The addition of the r_base version specification is a good practice for ensuring reproducibility and consistency in the build process.
🧰 Tools
🪛 yamllint
[error] 2-2: no new line character at the end of file
(new-line-at-end-of-file)
1-2: Minor improvements and version verification needed.
- Add a newline at the end of the file for better compatibility with text processing tools.
- Verify if r_base version 3.6.3 is the most appropriate for the r-saccharis package.
To address these points:
- Add a newline at the end of the file:
r_base: - 3.6.3 +
- Let's verify the r_base version:
Please review the script output to ensure the specified r_base version is appropriate for r-saccharis and consistent with other recipes in the repository.
✅ Verification successful
Minor improvements addressed and r_base version verified.
- Added a newline at the end of the file for better compatibility.
- Verified that the specified
r_baseversion (3.6.3) is consistent with other recipes in the repository.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if there's a more recent r_base version used in other recipes # and verify the specified version against the r-saccharis package requirements. # Check for more recent r_base versions in other recipes echo "Recent r_base versions used in other recipes:" rg -g 'recipes/*/conda_build_config.yaml' 'r_base:' | sort -V | tail -n 5 # Check r-saccharis package for any specific R version requirements echo "R version requirements in r-saccharis package:" rg -g 'recipes/r-saccharis/meta.yaml' 'r-base' -A 3Length of output: 955
Script:
#!/bin/bash # Description: Extract r_base versions from all conda_build_config.yaml files # and compare them to determine if 3.6.3 is the most appropriate version. # Extract r_base versions from all recipes echo "Extracted r_base versions from other recipes:" rg -g 'recipes/*/conda_build_config.yaml' '^r_base:\s*-\s*(\S+)' --only-matching --replace '$1' | sort -V | uniq # Determine the latest r_base version latest_version=$(rg -g 'recipes/*/conda_build_config.yaml' '^r_base:\s*-\s*(\S+)' --only-matching --replace '$1' | sort -V | tail -n 1) echo "Latest r_base version used in recipes: $latest_version" # Compare with the specified version specified_version="3.6.3" if [ "$(printf '%s\n' "$latest_version" "$specified_version" | sort -V | tail -n1)" = "$latest_version" ] && [ "$latest_version" != "$specified_version" ]; then echo "A more recent r_base version ($latest_version) is used in other recipes. Consider updating to this version." else echo "The specified r_base version ($specified_version) is up-to-date with other recipes." fiLength of output: 692
Script:
#!/bin/bash # Description: Extract all instances of r_base from conda_build_config.yaml files, # including those without specified versions, to determine how r_base is defined across recipes. echo "All r_base entries from conda_build_config.yaml files:" rg -g 'recipes/*/conda_build_config.yaml' '^r_base:' -A 2 echo "Detailed r_base version information:" rg -g 'recipes/*/conda_build_config.yaml' '^r_base:\s*-\s*(\S+)?' --only-matching | sort | uniqLength of output: 1366
🧰 Tools
🪛 yamllint
[error] 2-2: no new line character at the end of file
(new-line-at-end-of-file)
recipes/r-saccharis/meta.yaml (3)
1-8: LGTM: Package metadata is correctly defined.The package version, name, and build number are properly set using Jinja2 templating, which is the correct approach for conda recipes. The
filehashvariable is also defined, which will be used later in the recipe.🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
15-22: LGTM: Build configuration is correct.The build section is well-configured:
noarch: genericis appropriate for R packages.- Build number is correctly set using the variable.
- RPaths are properly specified for R libraries.
- The
run_exportssection syntax is correct, addressing the previous review comment.
1-63: Overall, the recipe is well-structured and ready for use.The
meta.yamlfile for the r-saccharis package is comprehensive and follows conda recipe best practices. It includes all necessary sections (package metadata, source, build configuration, dependencies, tests, and about information) required for building and distributing the package.Minor suggestions have been made throughout the review to enhance maintainability, robustness, and adherence to standards. These include:
- Adding comments for clarity
- Considering version constraints for dependencies
- Expanding test coverage
- Updating license information to use SPDX identifiers
With these minor improvements, the recipe will be even more robust and maintainable. Great job on creating a solid conda recipe for r-saccharis!
🧰 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/r-saccharis/meta.yaml (2)
6-13: LGTM: Package and source information correctly defined.The package name, version, and source information are well-defined. The use of Jinja2 variables ensures consistency across the file.
Consider adding a comment above the
filehashvariable (line 4) to indicate that it should be updated with each new release. This can help maintainers remember to update this crucial piece of information.
52-54: LGTM: Basic test included, but could be expanded.The test command provides a basic check to ensure the package can be loaded, which is good. However, consider expanding the tests for more comprehensive coverage.
Consider adding more detailed tests, such as:
- Checking if key functions from the package are available.
- Running a simple example that uses the package's functionality.
This would provide greater confidence in the package's correct installation and functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/r-saccharis/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/r-saccharis/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (5)
recipes/r-saccharis/meta.yaml (5)
1-4: LGTM: Package metadata correctly defined.The package version, name, build number, and file hash are correctly defined using Jinja2 variables. This setup allows for easy updates and maintenance of the package metadata.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
15-22: LGTM: Build section correctly configured.The build section is well-configured:
noarch: genericis appropriate for this R package.- RPaths are correctly set for R libraries.
- The
run_exportssection syntax has been fixed, addressing the issue mentioned in past review comments.The syntax issue in the
run_exportssection has been resolved, as suggested in the previous review.
24-50: LGTM: Requirements well-defined, but clarification needed on r-magicfor.The host and run requirements are well-specified and appropriate for an R package. However, there's a discrepancy with
r-magicfor:
- It's commented out in the host requirements (line 30).
- It's included in the run requirements (line 43).
Could you clarify why
r-magicforis treated differently in host and run requirements? If it's needed at runtime, it might also be necessary during the build process.
56-64: LGTM: Comprehensive package information provided.The about section provides a thorough overview of the package:
- The home page, license, summary, and description are well-defined.
- The license file is correctly specified.
- The description offers clear instructions on package usage.
This information is valuable for users and maintainers alike.
1-1: False positive: Ignore yamllint syntax error.The static analysis tool (yamllint) reports a syntax error for the '%' character at the beginning of the file. This is a false positive as these '%' characters are part of the Jinja2 templating syntax, which is commonly and correctly used in Conda recipe files.
No action is needed to address this reported issue.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
maybe r channel addition should be in conda_build_config instead?
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/r-saccharis/pre-link.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck
recipes/r-saccharis/pre-link.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
🔇 Additional comments (1)
recipes/r-saccharis/pre-link.sh (1)
1-1: LGTM! Verify the impact on the conda environment.The addition of the 'r' channel to the conda configuration aligns with the PR objective of adding the r-saccharis package to Bioconda. This change is crucial for proper package resolution.
To ensure this change doesn't have unintended consequences, please run the following verification script:
This script will help ensure that the addition of the 'r' channel doesn't negatively impact the conda environment and that essential R packages are available.
🧰 Tools
🪛 Shellcheck
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
| @@ -0,0 +1 @@ | |||
| conda config --append channels r | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving error handling and idempotency.
While the current implementation works, it could be improved to handle errors and avoid duplicate entries in the conda config.
Consider refactoring the script as follows:
#!/bin/bash
set -e
if ! conda config --get channels | grep -q '^ - r$'; then
conda config --append channels r
else
echo "The 'r' channel is already in the conda configuration."
fiThis refactored version:
- Uses
set -eto exit on any error. - Checks if the 'r' channel already exists before adding it.
- Provides feedback if the channel is already present.
🧰 Tools
🪛 Shellcheck
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
Add a shebang to improve portability.
The script is missing a shebang, which could lead to inconsistent behavior across different environments.
Add the following shebang at the beginning of the file:
+#!/bin/bash
conda config --append channels r📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| conda config --append channels r | |
| #!/bin/bash | |
| conda config --append channels r |
🧰 Tools
🪛 Shellcheck
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
Add the r-saccharis phylogenetic tree rendering package for SACCHARIS output to bioconda.
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>.