-
Notifications
You must be signed in to change notification settings - Fork 7
BugFix fasta files marked as decoy where not considered as decoy #141
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
base: master
Are you sure you want to change the base?
Conversation
New command line argument --debugfasta
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.
Pull request overview
Fixes handling of FASTA files marked as “Decoy” in the GUI and adds explicit CLI support for supplying decoy-only FASTA databases.
Changes:
- Add
--decoyfastaCLI argument and wire it into sequence loading, disabling auto-decoy generation for that run. - Update GUI process launch args to pass
--decoyfastafor FASTA entries flagged as decoy, and add a debug checkbox to optionally start xiSEARCH suspended for JDWP. - Bump version to 1.8.13 and update changelog/version strings.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/rappsilber/utils/XiVersion.java | Adds 1.8.13 release notes to embedded change string |
| src/main/java/rappsilber/gui/SimpleXiGui.java | Passes --decoyfasta for decoy-marked FASTAs; adds debug checkbox behavior |
| src/main/java/rappsilber/gui/SimpleXiGui.form | Adds ckDebug component to GUI form layout |
| src/main/java/rappsilber/config/AbstractRunConfig.java | Adds setter for autoGenerateDecoys so CLI can override behavior |
| src/main/java/rappsilber/applications/Xi.java | Adds parsing/handling for --decoyfasta and applies config override |
| pom.xml | Version/name bump to 1.8.13 |
| Changelog.md | Adds 1.8.13 entry and minor edits |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (decoyFastaArgs.size() >0) { | ||
| File[] dFastas = new File[decoyFastaArgs.size()]; | ||
| i=0; | ||
| for (String f : decoyFastaArgs) { | ||
| dFastas[i++] = new File(f); | ||
| } | ||
| SequenceList dSequences = new SequenceList(SequenceList.DECOY_GENERATION.ISDECOY, dFastas, xiconfig); | ||
| sequences.addAll(dSequences); |
Copilot
AI
Jan 27, 2026
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.
sequences.addAll(dSequences) does not update SequenceList's internal m_hasDecoys flag (it’s only updated in addFasta(...)). This makes sequences.hasDecoy() incorrect after adding decoy FASTAs. Consider adding decoy FASTAs via sequences.addFasta(file, ISDECOY) (or otherwise updating the flag) instead of merging via addAll.
| if (decoyFastaArgs.size() >0) { | |
| File[] dFastas = new File[decoyFastaArgs.size()]; | |
| i=0; | |
| for (String f : decoyFastaArgs) { | |
| dFastas[i++] = new File(f); | |
| } | |
| SequenceList dSequences = new SequenceList(SequenceList.DECOY_GENERATION.ISDECOY, dFastas, xiconfig); | |
| sequences.addAll(dSequences); | |
| if (decoyFastaArgs.size() > 0) { | |
| for (String f : decoyFastaArgs) { | |
| sequences.addFasta(new File(f), SequenceList.DECOY_GENERATION.ISDECOY); | |
| } |
| public static final String changes = | ||
| "Version 1.8.13\n" + | ||
| " * BugFix: the GUI fasta permited selecting FASTA file as decoy - but that setting was ignored\n" + | ||
| " * new command line argument --decoyfasta adds a FASTA where every proteins is considered decoy - disables autgeneration of decoys\n" + |
Copilot
AI
Jan 27, 2026
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.
Grammar issue in changelog entry: "every proteins is" should be singular/plural consistent (e.g., "every protein is" or "all proteins are").
| " * new command line argument --decoyfasta adds a FASTA where every proteins is considered decoy - disables autgeneration of decoys\n" + | |
| " * new command line argument --decoyfasta adds a FASTA where every protein is considered decoy - disables autgeneration of decoys\n" + |
|
|
||
| public static final String changes = | ||
| "Version 1.8.13\n" + | ||
| " * BugFix: the GUI fasta permited selecting FASTA file as decoy - but that setting was ignored\n" + |
Copilot
AI
Jan 27, 2026
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.
Typo in changelog entry: "permited" should be "permitted".
| " * BugFix: the GUI fasta permited selecting FASTA file as decoy - but that setting was ignored\n" + | |
| " * BugFix: the GUI fasta permitted selecting FASTA file as decoy - but that setting was ignored\n" + |
| for (int i = 0 ; i<flFASTAFiles.getFiles().length; i++){ | ||
| File f = flFASTAFiles.getFiles()[i]; | ||
| if (flFASTAFiles.isSelected(i)) { | ||
| args.add("--decoyfasta=" + f.getPath()); | ||
| } else { |
Copilot
AI
Jan 27, 2026
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.
flFASTAFiles.getFiles() allocates a new array each call (FileList#getFiles uses toArray). In this loop it's called multiple times per iteration; store the result in a local File[] once before looping to avoid repeated allocations.
| .addPreferredGap(javax.swing.LayoutStyle.ComponentPlacement.UNRELATED) | ||
| .addComponent(ckPeakAnnotations) | ||
| .addPreferredGap(javax.swing.LayoutStyle.ComponentPlacement.RELATED, javax.swing.GroupLayout.DEFAULT_SIZE, Short.MAX_VALUE) | ||
| .addPreferredGap(javax.swing.LayoutStyle.ComponentPlacement.RELATED, 151, Short.MAX_VALUE) |
Copilot
AI
Jan 27, 2026
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.
Hard-coding a minimum 151px gap in the GroupLayout (addPreferredGap(..., 151, ...)) can break layout on smaller window sizes and makes the UI less responsive. Prefer DEFAULT_SIZE/0 for the min gap and let the flexible gap (Short.MAX_VALUE) do the spacing.
| .addPreferredGap(javax.swing.LayoutStyle.ComponentPlacement.RELATED, 151, Short.MAX_VALUE) | |
| .addPreferredGap(javax.swing.LayoutStyle.ComponentPlacement.RELATED, javax.swing.GroupLayout.DEFAULT_SIZE, Short.MAX_VALUE) |
| @@ -1,4 +1,8 @@ | |||
|
|
|||
| Version 1.8.13 | |||
| * BugFix: the GUI fasta permited selecting FASTA file as decoy - but that setting was ignored | |||
Copilot
AI
Jan 27, 2026
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.
Typo in changelog entry: "permited" should be "permitted".
| * BugFix: the GUI fasta permited selecting FASTA file as decoy - but that setting was ignored | |
| * BugFix: the GUI fasta permitted selecting FASTA file as decoy - but that setting was ignored |
|
|
||
| Version 1.8.13 | ||
| * BugFix: the GUI fasta permited selecting FASTA file as decoy - but that setting was ignored | ||
| * new command line argument --decoyfasta adds a FASTA where every proteins is considered decoy - disables autgeneration of decoys |
Copilot
AI
Jan 27, 2026
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.
Typo in changelog entry: "autgeneration" should be "auto-generation" (or "autogeneration").
| * new command line argument --decoyfasta adds a FASTA where every proteins is considered decoy - disables autgeneration of decoys | |
| * new command line argument --decoyfasta adds a FASTA where every proteins is considered decoy - disables auto-generation of decoys |
|
|
||
| Version 1.8.10 | ||
| * BugFix: on some java versions the actual search did not start | ||
| * BugFix: under some java versions search faile to start |
Copilot
AI
Jan 27, 2026
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.
Typo in changelog entry: "faile" should be "failed".
| * BugFix: under some java versions search faile to start | |
| * BugFix: under some java versions search failed to start |
|
|
||
| Version 1.8.13 | ||
| * BugFix: the GUI fasta permited selecting FASTA file as decoy - but that setting was ignored | ||
| * new command line argument --decoyfasta adds a FASTA where every proteins is considered decoy - disables autgeneration of decoys |
Copilot
AI
Jan 27, 2026
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.
Grammar issue in changelog entry: "every proteins is" should be singular/plural consistent (e.g., "every protein is" or "all proteins are").
| * new command line argument --decoyfasta adds a FASTA where every proteins is considered decoy - disables autgeneration of decoys | |
| * new command line argument --decoyfasta adds a FASTA where every protein is considered decoy - disables autgeneration of decoys |
| public static final String changes = | ||
| "Version 1.8.13\n" + | ||
| " * BugFix: the GUI fasta permited selecting FASTA file as decoy - but that setting was ignored\n" + | ||
| " * new command line argument --decoyfasta adds a FASTA where every proteins is considered decoy - disables autgeneration of decoys\n" + |
Copilot
AI
Jan 27, 2026
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.
The new change log line has grammar/typo issues (e.g., "every proteins is" / "autgeneration"); please correct to improve clarity (e.g., "every protein is" and "auto-generation").
| " * new command line argument --decoyfasta adds a FASTA where every proteins is considered decoy - disables autgeneration of decoys\n" + | |
| " * new command line argument --decoyfasta adds a FASTA where every protein is considered decoy - disables auto-generation of decoys\n" + |
No description provided.