-
Notifications
You must be signed in to change notification settings - Fork 0
New features #4
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
New features #4
Conversation
Reviewer's GuideThis PR refactors the peptide plotting routine to handle empty groups with consistent axes rather than hiding them, adds debugging scaffolding for group extraction, updates the runner to use structured logging and pass explicit group context, introduces console log persistence hooks in the logger, and synchronizes the README’s example image with the latest identifier. Sequence diagram for peptide group plotting with empty group handlingsequenceDiagram
participant User
participant Runner as run_pipeline
participant Viz as plot_peptigram
participant Matplotlib as ax
User->>Runner: run_pipeline()
Runner->>Viz: plot_peptigram(proteins, groups, ...)
Viz->>Viz: For each group
alt Group has peptides
Viz->>Matplotlib: Plot peptides with color and layout
else Group is empty
Viz->>Matplotlib: Set xlim, ylim for empty plot
end
Viz->>Runner: Return figure
Runner->>User: Output visualization
Class diagram for logging and plotting changesclassDiagram
class Logger {
+configure_logging(level, log_to_file)
+save_console_log(output_dir, file_name)
}
class Runner {
+run_pipeline(...)
}
class Viz {
+plot_peptigram(...)
-_calculate_plot_height(...)
}
Logger <.. Runner : uses
Runner <.. Viz : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @sanjaysgk - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `ProtPeptigram/viz.py:573` </location>
<code_context>
- space_start = max(0, start - xlim[0])
- space_end = min(end - xlim[0], xlim[1] - xlim[0])
-
- if space_start >= spaces.shape[1] or space_end >= spaces.shape[1]:
- continue
-
- space_needed = spaces[height, space_start:space_end+1]
</code_context>
<issue_to_address>
Skipping peptides with out-of-bounds indices may silently drop data.
Currently, peptides outside the plotting range are skipped without notification, which can hinder debugging. Please add a warning or indicator when peptides are omitted for this reason.
Suggested implementation:
```python
+ space_start = max(0, start - xlim[0])
+ space_end = min(end - xlim[0], xlim[1] - xlim[0])
+
+ if space_start >= spaces.shape[1] or space_end >= spaces.shape[1]:
+ import warnings
+ warnings.warn(
+ f"Peptide ({start}, {end}) at height {height} is out of plotting range ({xlim[0]}, {xlim[1]}) and will be skipped.",
+ UserWarning
+ )
+ continue
```
If the `warnings` module is already imported at the top of the file, you can remove the inline `import warnings` and use the existing import. Otherwise, for best practice, add `import warnings` at the top of the file.
</issue_to_address>
### Comment 2
<location> `README.md:12` </location>
<code_context>
ProtPeptigram provides a comprehensive visualization platform for mapping immunopeptides to their source proteins across different biological samples. This tool can enables to identify peptide coverage patterns, analyze density distributions, and compare peptide presentations between experimental conditions.
<p align="center">
</code_context>
<issue_to_address>
Grammatical error: 'can enables to identify' should be corrected.
Consider revising to 'can enable identification of', 'enables identification of', or 'can enable users to identify' for clarity and correctness.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
ProtPeptigram provides a comprehensive visualization platform for mapping immunopeptides to their source proteins across different biological samples. This tool can enables to identify peptide coverage patterns, analyze density distributions, and compare peptide presentations between experimental conditions.
=======
ProtPeptigram provides a comprehensive visualization platform for mapping immunopeptides to their source proteins across different biological samples. This tool enables identification of peptide coverage patterns, analysis of density distributions, and comparison of peptide presentations between experimental conditions.
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -480,119 +483,121 @@ def plot_peptigram( | |||
| lambda x: group in x)] | |||
|
|
|||
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.
suggestion (bug_risk): Skipping peptides with out-of-bounds indices may silently drop data.
Currently, peptides outside the plotting range are skipped without notification, which can hinder debugging. Please add a warning or indicator when peptides are omitted for this reason.
Suggested implementation:
+ space_start = max(0, start - xlim[0])
+ space_end = min(end - xlim[0], xlim[1] - xlim[0])
+
+ if space_start >= spaces.shape[1] or space_end >= spaces.shape[1]:
+ import warnings
+ warnings.warn(
+ f"Peptide ({start}, {end}) at height {height} is out of plotting range ({xlim[0]}, {xlim[1]}) and will be skipped.",
+ UserWarning
+ )
+ continueIf the warnings module is already imported at the top of the file, you can remove the inline import warnings and use the existing import. Otherwise, for best practice, add import warnings at the top of the file.
| @@ -12,7 +12,7 @@ | |||
| ProtPeptigram provides a comprehensive visualization platform for mapping immunopeptides to their source proteins across different biological samples. This tool can enables to identify peptide coverage patterns, analyze density distributions, and compare peptide presentations between experimental conditions. | |||
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.
issue (typo): Grammatical error: 'can enables to identify' should be corrected.
Consider revising to 'can enable identification of', 'enables identification of', or 'can enable users to identify' for clarity and correctness.
| ProtPeptigram provides a comprehensive visualization platform for mapping immunopeptides to their source proteins across different biological samples. This tool can enables to identify peptide coverage patterns, analyze density distributions, and compare peptide presentations between experimental conditions. | |
| ProtPeptigram provides a comprehensive visualization platform for mapping immunopeptides to their source proteins across different biological samples. This tool enables identification of peptide coverage patterns, analysis of density distributions, and comparison of peptide presentations between experimental conditions. |
Summary by Sourcery
Improve peptigram plotting to handle empty groups as empty rows, accept explicit group lists, standardize console logging, and refresh the README example image.
Enhancements:
Documentation:
Chores: