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.

Sign up for GitHub

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

Jump to bottom

Initial add telosearchlr-v1.0.0 #51438

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
+47 −0
Open

Initial add telosearchlr-v1.0.0 #51438

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
48fdb59
Initial add telosearchlr-v1.0.0
jayramr Oct 16, 2024
cd99797
build custom telosearchlr-v1.0.0
jayramr Oct 17, 2024
fb5d7af
build custom telosearchlr-v1.0.0
jayramr Oct 17, 2024
f059587
build custom telosearchlr-v1.0.0
jayramr Oct 17, 2024
c168153
build custom telosearchlr-v1.0.0
jayramr Oct 17, 2024
6964d90
build custom telosearchlr-v1.0.0
jayramr Oct 17, 2024
19bd23e
build custom telosearchlr-v1.0.1
jayramr Oct 18, 2024
69b7c15
build custom telosearchlr-v1.0.1
jayramr Oct 18, 2024
9c1473d
Merge branch 'master' into telosearchlr-v1.0.0
jayramr Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions recipes/telosearchlr/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#!/bin/bash
mkdir -p ${PREFIX}/bin
for f in *.py;
do
if ! grep -q "^#!/usr/bin/env python3" "$f"; then
sed -i "1i #!/usr/bin/env python3" "$f"
fi
done
chmod +x *.py
mv *.py $PREFIX/bin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Restrict moving only executable Python scripts to $PREFIX/bin.

The current mv *.py $PREFIX/bin command moves all Python files, including non-executable modules and libraries, to the $PREFIX/bin directory. This can lead to unnecessary clutter and potential conflicts in the system-wide bin.

Recommended Action:

  • Modify the mv command to target only Python files with a shebang line. For example: 
    grep -l '^#!/usr/bin/env python' *.py | xargs mv -t $PREFIX/bin
    This ensures that only executable Python scripts are moved to $PREFIX/bin, while non-executable modules and libraries remain in their original locations.
🔗 Analysis chain

Verify the inclusion of all Python files as executables.

The script correctly moves all Python files to the $PREFIX/bin directory, making them accessible system-wide after installation. This is consistent with conda build practices.

However, please verify that all Python files in the package are indeed meant to be executable scripts. If there are any modules or libraries that should not be in the PATH, consider modifying the script to move only specific files or to use a different destination for non-executable Python files.

To help with this verification, you can run the following command in the package source directory:

This will help identify which Python files are designed as executable scripts and which might be modules or libraries.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# List all Python files in the current directory along with their first line
# to check if they have a proper shebang for executable scripts

for file in *.py; do
    echo "File: $file"
    head -n 1 "$file"
    echo "---"
done

Length of output: 147

Script:

#!/bin/bash
# Find all Python files in the repository and display their first line to identify executables

find . -type f -name "*.py" -exec echo "File: {}" \; -exec head -n 1 {} \; -exec echo "---" \;

Length of output: 11734

38 changes: 38 additions & 0 deletions recipes/telosearchlr/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{% set name = "telosearchlr" %}
{% set version = "1.0.0" %}

package:
name: "{{ name|lower }}"
version: "{{ version }}"

source:
url: "https://github.com/gchchung/TeloSearchLR/archive/refs/tags/v1.0.0.tar.gz"
sha256: "37426e76cae03254a7746becd6b3174f9e0476ef13656998b93f036ec9468101"
patches:
- python.patch
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Patch file python.patch lacks explanation

The python.patch modifies sys.exit calls in TeloSearchLR.py:

- sys.exit(2)
+ sys.exit(0)

However, the purpose of these changes is not documented. Please provide an explanation for the modifications made by this patch to ensure clarity and maintainability.

🔗 Analysis chain

Source section looks good, but patch file needs explanation

The source URL and SHA256 checksum are correctly specified, which is excellent for reproducibility. However, there's a patch file referenced (python.patch) without any explanation of its purpose.

Could you please provide information about the purpose of the python.patch file? It's important to understand what modifications are being made to the original source code.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the content of the python.patch file
cat recipes/telosearchlr/python.patch

Length of output: 1242

Script:

#!/bin/bash
# Description: Attempt to display the python.patch file and search for its location if not found

# Attempt to display the correct python.patch file
if [ -f recipes/telosearchlr/python.patch ]; then
  cat recipes/telosearchlr/python.patch
else
  echo "recipes/telosearchlr/python.patch not found. Searching for python.patch in the repository..."
  fd --type f python.patch
fi

Length of output: 493


build:
noarch: generic
number: 0
run_exports:
- {{ pin_subpackage('telosearchlr', max_pin="x") }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider revising the build section for a Python package.

While the run_exports section is correctly implemented, there are a couple of points to consider:

  1. The use of noarch: generic is unusual for a Python package. For a pure Python package, it's recommended to use noarch: python instead.
  2. The build section is missing a script for installing the package.

Consider replacing the current build section with:

build:
  noarch: python
  number: 0
  script: "{{ PYTHON }} -m pip install . -vv"
  run_exports:
    - {{ pin_subpackage('telosearchlr', max_pin="x") }}

This change to noarch: python is more appropriate for a pure Python package and allows for the use of the standard Python build process. The script line ensures that the package is installed using pip, which is the recommended method for Python packages in Conda.


requirements:
host:
- python >=3.9
run:
- python >=3.9
- Bio
- pillow
- svgutils
- plotly
- tidehunter

test:
commands:
- TeloSearchLR.py -h
Comment on lines +29 to +31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding more comprehensive tests

While the current test command ensures that the main script is accessible and can be executed, it doesn't verify the actual functionality of the package. Consider adding more comprehensive tests to ensure the package works as expected.

For example, you could add a simple test case that processes a small sample dataset, if available:

test:
  commands:
    - TeloSearchLR.py -h
    - TeloSearchLR.py --version
    - python -c "import telosearchlr; assert telosearchlr.__version__ == '{{ version }}'"
  # If a sample dataset is available:
  # - TeloSearchLR.py -i sample_data.fastq -o test_output

This would provide more confidence in the package's functionality.

about:
home: "https://github.com/gchchung/TeloSearchLR"
license: "Academic and Non-Commercial Research Use"
license_file: LICENSE
summary: "TeloSearchLR (telomere search using long sequencing reads) is a Python script for aiding the identificaiton of telomeric repeat motifs."
Comment on lines +32 to +36
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

License Specification Mismatch

  • The meta.yaml file specifies the license as "Academic and Non-Commercial Research Use", but the LICENSE file contains the MIT License. Please ensure that the license in meta.yaml accurately reflects the project's actual license.
🔗 Analysis chain

Please clarify the license and fix the typo in the summary

  1. The license "Academic and Non-Commercial Research Use" is somewhat unusual. Please confirm if this is the official license name. If possible, consider using a standard SPDX license identifier or provide a link to the full license text for clarity.

  2. There's a typo in the summary: "identificaiton" should be "identification".

Please correct the typo in the summary:

summary: "TeloSearchLR (telomere search using long sequencing reads) is a Python script for aiding the identification of telomeric repeat motifs."

To verify the license, please run the following script:

This will help ensure that the license in the meta.yaml file accurately reflects the project's actual license.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the content of the LICENSE file
cat LICENSE

Length of output: 1103

11 changes: 11 additions & 0 deletions recipes/telosearchlr/python.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
--- TeloSearchLR.py-old 2024-10-17 23:03:55.738331016 +0400
+++ TeloSearchLR.py 2024-10-17 23:04:03.533242382 +0400
@@ -818,7 +818,7 @@
for opt, arg in opts:
if opt in ("-h", "--help"):
print(arg_help) # print the help message
- sys.exit(2)
+ sys.exit(0)
elif opt in ("-v", "--version"):
print(version_number)
sys.exit(2)
Loading