Update pretextgraph #51674
Update pretextgraph #51674
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces modifications to two files: Suggested labels
📜 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: 1
🧹 Outside diff range and nitpick comments (2)
recipes/pretextgraph/build.sh (1)
Line range hint
1-14: Consider adding error handling for the build process.While the script uses
set -eo pipefail, it would be beneficial to add explicit error handling around the meson commands to provide more informative error messages during build failures.Consider wrapping the build commands in a function with error handling:
#!/bin/bash set -eo pipefail +build_and_install() { + if ! meson compile; then + echo "Error: Build failed" >&2 + return 1 + fi + + if ! meson test; then + echo "Error: Tests failed" >&2 + return 1 + fi + + if ! meson install; then + echo "Error: Installation failed" >&2 + return 1 + fi +} + if [ `uname` == Darwin ]; then meson setup --buildtype=release --prefix=$PREFIX --unity on builddir else env CC=clang CXX=clang++ meson setup --buildtype=release --prefix=$PREFIX --unity on builddir fi cd builddir -meson compile -meson test -meson install +build_and_install🧰 Tools
🪛 Shellcheck
[warning] 5-5: Quote this to prevent word splitting.
(SC2046)
recipes/pretextgraph/meta.yaml (1)
Line range hint
27-36: Update repository references in about sectionThe home URL and description still reference the old repository (wtsi-hpag) while the source now points to sanger-tol. Consider updating these references for consistency.
Apply this diff:
- home: https://github.com/wtsi-hpag/PretextGraph + home: https://github.com/sanger-tol/PretextGraph description: > This is a tool for converting data from a bedgraph format and embedding into a Hi-C contact map in the Pretext format. The graph data can then be displayed alongside the contact map using the PretextView desktop software (https://github.com/wtsi-hpag/PretextView). - See https://github.com/wtsi-hpag/PretextMap for how to generate Pretext contact maps, or search for pretextmap on bioconda. + See https://github.com/sanger-tol/PretextMap for how to generate Pretext contact maps, or search for pretextmap on bioconda.🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/pretextgraph/build.sh (1 hunks)
- recipes/pretextgraph/meta.yaml (1 hunks)
🔇 Additional comments (3)
recipes/pretextgraph/build.sh (1)
8-8: LGTM! Compiler change aligns with C++ codebase requirements.The change from
clangtoclang++is appropriate given the C++ code modifications mentioned in the PR objectives.Let's verify the C++ file extensions in the codebase:
✅ Verification successful
Let's verify the PretextGraph repository specifically since the current results show files from various recipes but we need to focus on the PretextGraph codebase.
Let's check the meta.yaml file to see if it references any source code or repository that would confirm the C++ nature of the project.
Compiler change is correct and verified by package requirements
The change from
clangtoclang++in build.sh is appropriate because:
- The meta.yaml explicitly requires a C++ compiler (
{{ compiler('cxx') }})- The package uses Meson build system which is commonly used for C++ projects
- The Linux-specific requirement for clang>=9.0.1 indicates C++ compilation needs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify presence of C++ source files to confirm compiler choice # Expected: Find .cpp/.cxx/.cc files indicating C++ source code # Search for C++ source files echo "Searching for C++ source files..." fd -e cpp -e cxx -e cc -e hpp -e hxx -e h # Verify if any files explicitly specify C++ requirements echo -e "\nChecking for C++ specific features..." rg -l "class\s+\w+|template\s*<|namespace\s+\w+"Length of output: 11636
Script:
#!/bin/bash # Check for C++ files specifically in pretextgraph recipe and its source echo "Checking PretextGraph source files..." fd . recipes/pretextgraph -t f # Look for C++ patterns specifically in PretextGraph files echo -e "\nChecking for C++ patterns in PretextGraph files..." rg -l "class\s+\w+|template\s*<|namespace\s+\w+" recipes/pretextgraph/Length of output: 368
Script:
#!/bin/bash # Check meta.yaml for source information echo "Content of meta.yaml:" cat recipes/pretextgraph/meta.yaml # Check if build.sh has any other C++ indicators echo -e "\nChecking build.sh for compiler usage:" cat recipes/pretextgraph/build.shLength of output: 1519
recipes/pretextgraph/meta.yaml (2)
2-2: LGTM: Version bump is appropriateThe version increment to 0.0.7 aligns with the significant changes mentioned in the PR objectives, including data type changes, segmentation fault fixes, and new functionality.
14-14: LGTM: Build number increment is appropriateThe build number increment follows conda packaging best practices.
recipes/pretextgraph/meta.yaml
Outdated
| git_url: https://github.com/sanger-tol/PretextGraph.git | ||
| git_tag: "{{ version }}" | ||
| git_submodules: True |
There was a problem hiding this comment.
💡 Codebase verification
Repository ownership and submodules verified - update needed
The repository does require submodules as evidenced by the .gitmodules file, which shows dependencies on:
- libdeflate: https://github.com/ebiggers/libdeflate
- wrapper: https://github.com/wtsi-hpag/SIMDExtensionWrapper
However, both repositories (wtsi-hpag/PretextGraph and sanger-tol/PretextGraph) are currently active and independent (not a fork relationship), suggesting a parallel development rather than an ownership transfer. The recipe should maintain the original wtsi-hpag source or clarify the relationship between these repositories.
🔗 Analysis chain
Verify repository ownership transfer and submodule dependencies
The source has been changed from wtsi-hpag releases to sanger-tol git repository. This appears to be an ownership transfer.
Let's verify the repository transfer and submodule requirements:
Let me generate another verification script to check for submodules more accurately and verify the repository relationship:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify repository ownership and submodules
# Test 1: Check if the old repository redirects to the new one
echo "Checking repository transfer:"
gh repo view wtsi-hpag/PretextGraph 2>&1 || true
# Test 2: Verify the existence and accessibility of the new repository
echo -e "\nVerifying new repository:"
gh repo view sanger-tol/PretextGraph
# Test 3: Check for submodules in the repository
echo -e "\nChecking for submodules:"
git ls-remote --recurse-submodules https://github.com/sanger-tol/PretextGraph.git | grep -i modules || echo "No submodules found"
Length of output: 9926
Script:
#!/bin/bash
# Check for .gitmodules file and submodule references
echo "Checking for .gitmodules and submodule configuration:"
git clone --depth 1 https://github.com/sanger-tol/PretextGraph.git temp_repo
cd temp_repo
if [ -f .gitmodules ]; then
echo "Found .gitmodules file:"
cat .gitmodules
else
echo "No .gitmodules file found"
fi
echo -e "\nChecking git config for submodules:"
git config --list | grep -i submodule || echo "No submodule configuration found"
echo -e "\nChecking repository relationship:"
gh api repos/sanger-tol/PretextGraph --jq '.fork, .parent.full_name, .source.full_name' || echo "Not a fork"
cd ..
rm -rf temp_repo
Length of output: 995
Update PretextGraph to 0.0.7
Fixes and Improvements:
graph->valuesby changing the calculation type from u32 to f32.data_typefor handling different methods of transforming thebedgraphfile intoGraph->values:gap: Sets the pixel value with numberindexforGraph->values[index]containing the gap to1.repeat_density: Normalizes the value of each bin as a percentage, reflecting the ratio of repeat pairs within that bin. For instance, if thebin_size=10,000andNbase pairs are marked as repeats, the ratio is calculated asN / 10,000.default: Retains the original calculation method.PrintStatus: Resolved small bugs that caused the program to crash.ProcessLinefunction: Added a check to ensure(u32) nThisBindoes not become negative. Originally, if a negative value is encountered, it will be set to the maximum allowable value foru32. Currently, it will be set as 0.