Extend compatibility checks on parameter settings to all four programs. #33
Conversation
|
getting to this today, sorry for the delay... |
I can contribute the fixes for this.... |
|
That would be awesome @bradfordcondon! I just cleaned up the merge conflict so hopefully we can get this merged ASAP :-) I'm working towards releasing a new version of the module today with all the awesome fixes + features :-) |
|
ok shouldnt take me too long.
edit seeing as how it was previously commented out everywhere im going to assume we want to phase it out entirely. |
|
I'm not sure there is a long term plan. @ekcannon originally commented it out until we handle it correctly so I'd be tempted to leave it in (or at least comments referring to it) so we remember to handle it properly? What are your thoughts? |
|
all done. I made a PR to ethy's branch to fix the issues i spotted. I could submit blast jobs for everything now. |
|
RE: culling: yes, take it out. Here's the definition: It seemed to make sense as a way of limiting results, but apparently didn't pan out as expected. I've forgotten the details as to why, unfortunately. It doesn't appear on the NCBI BLAST UI. |
|
isnt culling = qrange? I assumed it was this:
You may be right about leaving in comments, i might have been overzealous removing them. We could add back in, or, alternatively, create an issue for later investigation. |
There was a problem hiding this comment.
Please see my PR ekcannon#1
A couple of stray options break the submissions for all types.
| @@ -521,9 +424,6 @@ function blast_ui_blastp_advanced_options_form_submit($form, $form_state) { | |||
| $gapKey = $form_state['values']['gapCost']; | |||
| $gap = _set_protein_gap($matrix, $gapKey); | |||
There was a problem hiding this comment.
set_protein_gap operates on the key (0, 1, 2) but it gets passed the value (ie 11_1) when submitting blastp job
There was a problem hiding this comment.
upon futher inpsection, i think that the protein submit wants to be using get_gap to explode the string based on the underscore.
| @@ -76,17 +68,6 @@ function blast_ui_blastn_advanced_options_form(&$form, $form_state) { | |||
| '#description' => t('The length of the seed that initiates an alignment'), | |||
| ); | |||
|
|
|||
| /*eksc: remove this as it is either the same as max_target_seqs, or miss-implemented | |||
| as culling_limit, which is something else entirely | |||
| $form['ALG']['GParam']['qRange'] = array( | |||
There was a problem hiding this comment.
qrange shows up in a few submissions. Looks like we just want to kill this parameter? It is a valid parameter on ncbi. DOn't get me wrong i don't care i just want the submits to pass without errors...
|
copying @ekcannon 's comment on qrange here to the core repo:
I second this approach. We can put the deleted code snippets in the issue if we really want. |
|
I third the suggestion of removing for now and creating an issue for it 👍 |
fix validator for proteins. remove unused parameters in validate.
|
Awesome :-) Merging this in and then I'll try to do a final test before the end of the day and make a new release! Thanks Team! |
This extends work started by @bradfordcondon. There is also a fair bit of code optimization and general cleanup.
Reference for determining compatible combinations of settings was the NCBI BLAST UI:
https://blast.ncbi.nlm.nih.gov/Blast.cgi