Skip to content

Add raw sequence predict#139

Merged
kathyxchen merged 4 commits intomasterfrom
add-raw-sequence-predict
Apr 23, 2020
Merged

Add raw sequence predict#139
kathyxchen merged 4 commits intomasterfrom
add-raw-sequence-predict

Conversation

@j-funk
Copy link
Copy Markdown
Collaborator

@j-funk j-funk commented Apr 23, 2020

What does this implement/fix? Explain your changes.

This PR modifies the AnalyzeSequences member method get_predictions to enable the submission of python strings as a raw sequence to retrieve predictions as Python object.

Previously predictions for sequences could only be requested using FASTA files.

What testing did you do to verify the changes in this PR?

I successfully retrieved an array of predictions using both a raw sequence of 1000 length, and using the same sequence formatted as a FASTA file. The resulting predictions were the same in both cases.

@j-funk j-funk requested review from aaronkw and kathyxchen April 23, 2020 19:40
Copy link
Copy Markdown
Collaborator

@kathyxchen kathyxchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @j-funk!! A couple minor comments, LGTM after those changes

Comment thread selene_sdk/predict/model_predict.py Outdated
r.write_to_file()

def _pad_or_truncate_sequence(self, input_seq):
sequence = input_seq
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this line, use directly _pad_or_truncate_sequence(self, sequence). thanks for the refactor, I like this helper function. :)

Comment thread selene_sdk/predict/model_predict.py Outdated
input : str
A single sequence, or a path to the FASTA or BED file input.
output_dir : str, optional
Output directory to write the model predictions. If this is left
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add default to the description. (Default is None. Output directory to...) Maybe also clarify that FASTA and BED inputs require an output directory to be specified?

@j-funk j-funk requested a review from kathyxchen April 23, 2020 20:12
@kathyxchen kathyxchen merged commit 97080e5 into master Apr 23, 2020
Copy link
Copy Markdown

@aaronkw aaronkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kathyxchen kathyxchen deleted the add-raw-sequence-predict branch July 13, 2020 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants