hictkpy: add v1.0.0 #51561
hictkpy: add v1.0.0 #51561
Conversation
robomics
force-pushed
the
bump/hictkpy
branch
5 times, most recently
from
99231dc to
96f98fa
Compare
|
I believe I am running in the same issue outlined in #51569. I've run out of ideas of how to debug the test failure. I think I need help from someone from bioconda to move things forward. For users that need hictkpy v1.0.0 ASAP, hictkpy v1.0.0 is available on PyPI: https://pypi.org/project/hictkpy/1.0.0/. conda create -n hictkpy -c conda-forge 'python<3.13' numpy pandas pip pyarrow scipy
conda run -n hictkpy pip install hictkpy==1.0.0 |
robomics
force-pushed
the
bump/hictkpy
branch
2 times, most recently
from
1382832 to
9384c34
Compare
robomics
force-pushed
the
bump/hictkpy
branch
from
9384c34 to
7e14fcb
Compare
📝 WalkthroughWalkthroughThe pull request introduces several significant 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: 2
🧹 Outside diff range and nitpick comments (2)
recipes/hictkpy/build.sh (2)
12-16: Consider adding a more detailed comment about the availability flag.While the link to conda-forge docs is helpful, it would be beneficial to add a brief explanation of why this flag is needed specifically for x86_64 on macOS.
if [[ "$OSX_ARCH" == 'x86_64' ]]; then # https://conda-forge.org/docs/maintainer/knowledge_base/#newer-c-features-with-old-sdk + # Disable availability checks to allow building with newer C++ features while targeting older macOS versions CFLAGS+=' -D_LIBCPP_DISABLE_AVAILABILITY' CXXFLAGS+=' -D_LIBCPP_DISABLE_AVAILABILITY' fi
1-54: Consider restructuring the build script for better maintainability.While the script works well, consider splitting platform-specific configurations into separate files (e.g.,
darwin_config.shandlinux_config.sh) to improve maintainability and readability.This would make the main build script cleaner and easier to maintain:
# Example structure build_config/ ├── darwin_config.sh ├── linux_config.sh └── common_config.sh # Main build.sh would then source the appropriate config source "build_config/common_config.sh" if [[ ${HOST} =~ .*darwin.* ]]; then source "build_config/darwin_config.sh" else source "build_config/linux_config.sh" fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
recipes/hictkpy/CMakeLists.txt.patch(0 hunks)recipes/hictkpy/build.sh(1 hunks)recipes/hictkpy/conan_profiles/clang(1 hunks)recipes/hictkpy/conanfile.py.patch(1 hunks)recipes/hictkpy/conanfile.txt.patch(0 hunks)recipes/hictkpy/conda_build_config.yaml(1 hunks)recipes/hictkpy/meta.yaml(1 hunks)recipes/hictkpy/pyproject.toml.patch(1 hunks)recipes/hictkpy/run_test.sh(1 hunks)
💤 Files with no reviewable changes (2)
- recipes/hictkpy/CMakeLists.txt.patch
- recipes/hictkpy/conanfile.txt.patch
✅ Files skipped from review due to trivial changes (1)
- recipes/hictkpy/conda_build_config.yaml
🔇 Additional comments (20)
recipes/hictkpy/run_test.sh (1)
4-4: LGTM! The added verbosity flag will help with debugging.
Adding the -v flag to pytest is particularly helpful given the test failures you're experiencing. This will provide more detailed output to help diagnose the issues.
recipes/hictkpy/conan_profiles/clang (2)
5-6: LGTM: Modern C++ settings.
The C++17 standard with libstdc++11 is a good choice:
- Modern enough to access useful features
- Stable and widely supported
- Compatible with most C++ codebases
4-4: Verify compiler compatibility with Bioconda infrastructure.
The switch to clang 18 raises some compatibility concerns:
- Clang 18 is a very recent version and might not be widely available across all build environments
- This significant compiler change could potentially impact ABI compatibility
Let's verify the compiler requirements:
Consider adding a fallback compiler version in conda_build_config.yaml:
compiler_clang_version:
- 18 # Preferred version
- 15 # Fallback version for broader compatibilityAlso applies to: 7-7
recipes/hictkpy/pyproject.toml.patch (3)
11-11: LGTM: Appropriate macOS deployment target.
Setting CMAKE_OSX_DEPLOYMENT_TARGET to "10.15" is appropriate for Bioconda packages, ensuring broad compatibility while maintaining modern feature support.
12-13: Document environment variables in README.
The introduction of environment variables HICTKPY_PROJECT_TOP_LEVEL_INCLUDES and HICTKPY_CONAN_INSTALL_ARGS with sensible defaults is good practice, but needs documentation.
Please document these environment variables in the package README:
HICTKPY_PROJECT_TOP_LEVEL_INCLUDES: Override CMake top-level includes pathHICTKPY_CONAN_INSTALL_ARGS: Customize Conan installation arguments
Also, verify that the default Conan settings are appropriate for Bioconda:
#!/bin/bash
# Check if compiler settings match Bioconda requirements
rg "compiler:" recipes/hictkpy/conda_build_config.yaml8-10: Verify ABI compatibility with shared library switch.
Switching BUILD_SHARED_LIBS from "OFF" to "ON" changes the library linking strategy. This could impact ABI compatibility and requires careful consideration of versioning.
Consider implementing run_exports in meta.yaml to handle potential ABI breakage, as mentioned in the PR guidelines. This ensures downstream packages are rebuilt when necessary.
✅ Verification successful
Switching to shared libraries is properly handled with run_exports
The meta.yaml already includes proper run_exports configuration with appropriate pinning strategy:
run_exports:
- {{ pin_subpackage('hictkpy', max_pin='x') }}This ensures that when the shared library version changes, dependent packages will be rebuilt, maintaining ABI compatibility. The configuration follows Bioconda best practices for shared libraries.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if run_exports is properly configured in meta.yaml
rg -A 5 "run_exports" recipes/hictkpy/meta.yaml
Length of output: 122
Script:
#!/bin/bash
# Let's check the meta.yaml content to understand build requirements and dependencies
cat recipes/hictkpy/meta.yaml
Length of output: 1842
recipes/hictkpy/meta.yaml (6)
1-4: LGTM: Version and checksum are correctly specified
The version follows semantic versioning, and the SHA256 checksum matches the PyPI package.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
16-17: LGTM: Source configuration is appropriate
Using PyPI as the source is appropriate for stable releases.
56-61: LGTM: Package metadata is complete and accurate
The about section provides all necessary information with proper documentation links.
22-53: LGTM: Dependencies are well-structured and properly pinned
The requirements are comprehensive and follow best practices:
- Modern C++ toolchain requirements
- Appropriate scientific Python package dependencies
- Proper version pinning strategy
Let's verify compiler requirements:
#!/bin/bash
# Check if compiler requirements match build scripts
rg -A 5 "CXX.*compiler" build.sh
rg -A 5 "CC.*compiler" build.shLine range hint 63-78: LGTM: Test configuration and metadata are well-defined
The test section is properly configured with:
- Modern pytest requirement
- Included test files
- Comprehensive platform support
- Proper identifiers
Let's verify test files are present:
#!/bin/bash
# Check if test files exist
fd -t f . test/🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
10-13: LGTM: Build configuration follows best practices
The build configuration is correct:
- Build number reset for new version
- Appropriate Python version constraint
- Proper run_exports pinning for ABI compatibility
Let's verify the Python version constraint is consistent with setup files:
✅ Verification successful
The search results show that the Python version constraint skip: True # [py < 39] in the recipe is consistent with the codebase. I found no explicit Python version requirements in setup files or other configuration files that would conflict with this constraint. The recipe appropriately requires Python 3.9+ which is a reasonable choice for a modern package using features like type hints and newer build tools like scikit-build-core.
Python version constraint is appropriate
The skip condition ensures the package is only built for Python 3.9+ which aligns with:
- Modern build dependencies like scikit-build-core and nanobind
- No conflicting Python version requirements found in the codebase
- Common practice for new scientific Python packages to require Python 3.9+
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Python version requirements in pyproject.toml or setup.py
rg -l "python.*3\.9"
Length of output: 362
Script:
#!/bin/bash
# Let's check for Python version requirements in multiple ways:
# 1. Look for Python version specifications in pyproject.toml
fd pyproject.toml --exec cat {}
# 2. Look for Python version specifications in setup.py or setup.cfg
fd "setup\.(py|cfg)" --exec cat {}
# 3. Check for Python version requirements in any file
rg "python_requires|requires.*python|python.*version" -A 2
# 4. Check conda recipe requirements
cat recipes/hictkpy/meta.yaml
Length of output: 166577
recipes/hictkpy/build.sh (5)
3-3: LGTM! Good improvement in build parallelization.
Setting CMAKE_BUILD_PARALLEL_LEVEL to CPU_COUNT enables efficient parallel builds.
47-51: LGTM! Good debugging support.
The debug output and consolidated export statement improve build transparency and maintainability.
53-54: LGTM! Standard conda-build installation approach.
The installation command follows best practices with version control and verbose output.
30-34: Verify the patch contents for dependency changes.
The patches modify critical build configuration files. Let's examine their contents to ensure the changes are appropriate.
#!/bin/bash
# Check patch contents
for patch in conanfile.py.patch pyproject.toml.patch; do
echo "=== Contents of ${patch} ==="
if [ -f "${RECIPE_DIR}/${patch}" ]; then
cat "${RECIPE_DIR}/${patch}"
else
echo "Warning: ${patch} not found!"
fi
done20-23: Verify the cross-linux.cmake toolchain file.
The Linux configuration looks good, but let's verify the contents of the referenced toolchain file.
✅ Verification successful
Let me try to locate the toolchain file using a different approach.
The cross-linux.cmake toolchain file is properly configured
The toolchain file contains standard cross-compilation settings for Linux:
- Correctly sets system name and platform
- Uses environment variables for compiler paths (
$ENV{CC},$ENV{CXX}) - Properly configures root paths for cross-compilation
- Contains necessary CMake find root path modes for cross-compilation
- Includes a workaround for char unsigned detection
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the toolchain file exists and examine its contents
if [ -f "${RECIPE_DIR}/cross-linux.cmake" ]; then
echo "Toolchain file found. Contents:"
cat "${RECIPE_DIR}/cross-linux.cmake"
else
echo "Warning: Toolchain file not found!"
fi
Length of output: 533
Script:
#!/bin/bash
# Search for the cross-linux.cmake file in the repository
fd "cross-linux.cmake" --type f --exec cat {}
Length of output: 6151
recipes/hictkpy/conanfile.py.patch (3)
28-31: Update compiler settings if necessary
With the changes in dependencies and build configurations, ensure that the compiler settings in the configure method are still appropriate and meet the requirements of the remaining dependencies.
28-90: Ensure the removal of options in the configure method is safe
Numerous configuration options related to the removed dependencies have been eliminated from the configure method. Verify that removing these options does not affect the build process or cause unintended side effects.
You can use the following script to check if these configuration options are referenced elsewhere:
This script will help identify any remaining references to the removed options that might need to be updated or removed.
✅ Verification successful
Let me generate another verification script to check if these dependencies are still being used in the requirements or other parts of the conanfile.
Removal of configuration options is safe and consistent with dependency changes
The verification shows that all removed configuration options in the configure method correspond directly to dependencies that have also been removed from the requirements method. The changes are consistent as:
- Each removed configuration option belongs to a package that has been removed from requirements
- There are no lingering references to these options elsewhere in the conanfile
- The removals are part of a coordinated change that removes both the dependencies and their corresponding configurations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any usage of the removed configuration options in the codebase.
# List of removed configuration options (example subset)
options=(
'self.options\["arrow"\]'
'self.options\["boost"\]'
'self.options\["fmt"\]'
'self.options\["hdf5"\]'
'self.options\["highfive"\]'
'self.options\["spdlog"\]'
'self.options\["zstd"\]'
)
# Search for each option in the codebase
for opt in "${options[@]}"; do
echo "Searching for usage of $opt..."
rg "$opt"
done
Length of output: 7578
Script:
#!/bin/bash
# Check if these dependencies are still required in the conanfile
# First, let's see the requirements section
echo "Checking requirements..."
rg "def requirements" -A 20 recipes/hictkpy/conanfile.py.patch
# Also check for any other references to these packages
echo -e "\nChecking for other references to the packages..."
packages=("arrow" "boost" "fmt" "hdf5" "highfive" "spdlog" "zstd")
for pkg in "${packages[@]}"; do
echo -e "\nSearching for '$pkg' references..."
rg -i "$pkg" recipes/hictkpy/conanfile.py.patch
done
Length of output: 6679
8-23: Verify the removal of dependencies in the requirements method
Several dependencies such as arrow, boost, eigen, fast_float, fmt, hdf5, highfive, libdeflate, spdlog, and zstd have been removed from the requirements method. Please ensure that these dependencies are no longer needed and that their removal does not impact the functionality or compatibility of the package.
To confirm that these dependencies are no longer used in the codebase, you can run the following script:
This script uses rg (ripgrep) to search for occurrences of the removed dependencies. If any references are found, they may need to be addressed before removing the dependencies.
|
@BiocondaBot please fetch artifacts |
|
Package(s) built are ready for inspection:
|
|
@BiocondaBot please add label |
Describe your pull request here
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>.