-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
fix: user-suffix?nstruct-suffix #54
Conversation
WalkthroughThis pull request updates the module version in the initialization file and enhances the suffix handling in the Rosetta command execution. The version number is incremented from "0.2.11-post.2" to "0.2.11-post.3". In addition, the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Rosetta
participant User
Caller->>Rosetta: Invoke setup_tasks_native(base_cmd)
Rosetta->>Rosetta: Check for "-suffix" in base_cmd
alt "-suffix found"
Rosetta->>Rosetta: Retrieve associated suffix value
Rosetta->>Rosetta: Remove "-suffix" and its value from base_cmd
Rosetta->>User: Display warning about merging suffix
else "No -suffix"
Rosetta->>Rosetta: Use empty suffix value
end
Rosetta->>Rosetta: Construct command for RosettaCmdTask with suffix
Rosetta->>Caller: Return updated command list
Possibly related PRs
Poem
✨ Finishing Touches
🪧 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 (
|
Here's the code health analysis summary for commits Analysis Summary
|
[ci skip]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/RosettaPy/rosetta.py (1)
160-174
: LGTM! Clean implementation of suffix handling.The changes effectively handle user-specified suffixes while maintaining backward compatibility. The warning message clearly communicates the suffix merging behavior.
Consider adding a comment explaining the suffix format pattern for future maintainers:
suffix = None +# Handle user-specified suffix that will be merged as "{user_suffix}_{index:05}" if "-suffix" in base_cmd_copy:
Also applies to: 180-180
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/RosettaPy/__init__.py
(1 hunks)src/RosettaPy/rosetta.py
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/RosettaPy/init.py
🔇 Additional comments (1)
src/RosettaPy/rosetta.py (1)
175-184
:❓ Verification inconclusive
Verify the impact on output file naming.
The changes affect how output files are named. Please ensure that:
- The modified suffix pattern works correctly with Rosetta's output file handling
- The score file naming remains compatible with analysis tools
- The changes don't break any downstream processing that relies on file naming patterns
Run the following script to check the output file naming patterns in the codebase:
🏁 Script executed:
#!/bin/bash # Description: Search for patterns related to output file naming and suffix usage # Search for output file patterns echo "Searching for output file patterns..." rg -A 2 "\.sc|\.pdb" --type py # Search for suffix usage in tests echo "Searching for suffix usage in tests..." rg -A 2 "-suffix" --type py --glob "*test*.py"Length of output: 19830
Review Update on Output File Naming Impact
The changes to the suffix handling and score file naming appear to integrate well overall. However, the verification script for suffix usage in tests returned an error, making it difficult to conclusively infer that all test cases are correctly accounting for the new suffix pattern. Please manually verify the following:
- Ensure that the modified suffix pattern (
f"{suffix or ''}_{i:05}"
) correctly concatenates the user-provided suffix and maintains compatibility with Rosetta’s internal file naming conventions.- Confirm that the score file naming (e.g.,
f"{self.job_id}.score.{i:05}.sc"
) remains compatible with downstream analysis tools.- Review test cases to ensure they properly validate suffix usage in output files. Specifically, check that tests expecting suffix handling are updated and passing.
Summary by CodeRabbit
Chores
New Features