add harpy view #166
add harpy view #166
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request involve modifications to several files within the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ViewFunction
User->>CLI: run view command
CLI->>ViewFunction: call view(directory, snakefile)
ViewFunction->>ViewFunction: check directory and files
alt --snakefile flag set
ViewFunction->>ViewFunction: search for .smk files
else no --snakefile flag
ViewFunction->>ViewFunction: search for .log files
end
ViewFunction->>ViewFunction: sort files by modification time
ViewFunction->>ViewFunction: check read permissions
ViewFunction->>ViewFunction: determine file type (gzipped or not)
ViewFunction->>User: display file content
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 (4)
resources/buildlocal.sh (1)
15-15: LGTM! Consider adding error message for pip install failureThe pip install command changes are good improvements:
- Editable install (
-e) is appropriate for local development- Disabling pip version check speeds up installation
Consider adding error handling:
-pip install --no-deps --disable-pip-version-check -e . && \ +pip install --no-deps --disable-pip-version-check -e . || { echo "Error: Failed to install package"; exit 1; } && \harpy/__main__.py (1)
Line range hint
19-75: Consider enhancing error handling and user feedback.Since this is a CLI tool for viewing logfiles and snakefiles:
- Ensure proper error messages when files don't exist or are inaccessible
- Consider adding colorized output for better readability
- Add support for file type detection and appropriate formatting
- Consider implementing file watching functionality for live log viewing
Would you like me to provide specific implementation suggestions for any of these enhancements?
harpy/view.py (2)
11-28: Consider improving help text formatting.The help text is comprehensive but could be more readable with consistent spacing. There's an extra blank line at line 17 and inconsistent spacing around the keyboard shortcuts table.
""" View a workflow log file or snakefile - This convenience command lets you view the latest workflow log file of a Harpy output directory. You can use `--snakefile` to view the workflow snakefile instead. Output is printed to the screen via `less` and accepts the typical keyboard shortcuts to navigate the output, e.g.: | key | function | | :---------------------- | :------------------------- | | `up arrow`/`down arrow` | scroll up/down | | `Page Up`/`Page Down` | scroll up/down, but faster | | `q` | exit | """
31-59: Consider using enum for error codes and more specific file patterns.
- The numeric error codes (0, 1, 2) could be more maintainable using an enum.
- The log file pattern
*.log*might match unintended files (e.g.,*.logo).Consider these improvements:
+from enum import Enum + +class ViewError(Enum): + SUCCESS = 0 + DIRECTORY_NOT_FOUND = 1 + FILES_NOT_FOUND = 2 + - err = 0 + err = ViewError.SUCCESS if snakefile: files = [i for i in glob.iglob(f"{directory}/workflow/*.smk")] err_dir = f"{directory}/workflow/" err_file = "snakefiles" if not os.path.exists(f"{directory}/workflow"): - err = 1 + err = ViewError.DIRECTORY_NOT_FOUND elif not files: - err = 2 + err = ViewError.FILES_NOT_FOUND else: - files = [i for i in glob.iglob(f"{directory}/logs/snakemake/*.log*")] + files = [i for i in glob.iglob(f"{directory}/logs/snakemake/*.log") + + glob.iglob(f"{directory}/logs/snakemake/*.log.gz")]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
harpy/__main__.py(3 hunks)harpy/view.py(1 hunks)pyproject.toml(0 hunks)resources/buildlocal.sh(1 hunks)
💤 Files with no reviewable changes (1)
- pyproject.toml
🔇 Additional comments (5)
resources/buildlocal.sh (1)
16-16: 🛠️ Refactor suggestion
Review cleanup command changes
The current cleanup approach has potential issues:
- Using
rm -rfwithout specific targets is risky - Removing
harpy.egg-infocleanup (commented out) might leave installation artifacts
Let's verify what files typically need cleanup:
Consider a more specific cleanup:
- rm -rf build #harpy.egg-info
+ rm -rf build *.egg-info __pycache__ *.pycharpy/__main__.py (3)
19-19: LGTM! Import changes are well-structured.
The import changes look good:
- Corrected import path for popgroup module
- Clean addition of the view module import
Also applies to: 21-21
75-75: LGTM! Command group organization is appropriate.
The view command is correctly categorized under "Other Commands" and maintains alphabetical sorting.
48-48: Verify view command implementation.
The command registration looks correct, but let's verify the view command implementation.
✅ Verification successful
The view command implementation has been thoroughly verified through the shell script results. I can now provide the final response:
View command implementation is well-structured and complete
The command is properly implemented with:
- Clear documentation and help text
- Required directory argument with path validation
- Optional --snakefile flag to toggle between log/snakefile viewing
- Proper error handling for missing directories/files and permissions
- Smart file selection (picks latest by modification time)
- User-friendly output formatting using less with syntax highlighting
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the view command implementation and its interface
# Check if view.py exists and contains the view command implementation
fd "view.py" --type f --exec cat {} \;
# Look for any usage of the view command in tests or documentation
rg -A 5 "def view"
Length of output: 3419
harpy/view.py (1)
1-10: LGTM! Well-organized imports and clear documentation.
The imports are appropriate for the functionality, and the module's docstring clearly describes its purpose.
Adds harpy view as a convenience method to view logfiles and snakefiles
Summary by CodeRabbit
Release Notes
New Features
viewcommand in the CLI to allow users to view the latest log or snakefile for a specified workflow directory, with options for error handling and file navigation.Bug Fixes
popgroupmodule in the CLI.Chores