Skip to content
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

Turning in MiniProject1 #5

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

ewesterhoff
Copy link

No description provided.

Copy link

@SeunginLyu SeunginLyu left a comment

Choose a reason for hiding this comment

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

Overall, it looks really good and you also went beyond what was required. I made a few stylistic suggestions for future reference. Keep up the good work!

@@ -1,16 +1,15 @@
# -*- coding: utf-8 -*-
"""
YOUR HEADER COMMENT HERE
First project for Olin Software Design Fall 2017

Choose a reason for hiding this comment

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

It might be more helpful to add some short description of what this project is about.

if nucleotide_inputs[i] == nucleotide:
complement = nucleotide_complements[i]
i += 1
return complement

Choose a reason for hiding this comment

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

I like how you implemented this function without using any if or else if statements. Now that you've learned what dictionaries are, you could do something like this as well :
nucleotide_complements' = {'A':'T', 'T':'C', 'C':'G', 'G':'C'}
return nucleotide_complements[nucleotide]

pair = get_complement(letter) #finds complement
reverse = reverse + pair
i += 1
return reverse

Choose a reason for hiding this comment

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

fancy one line code (pythonic stylistic suggestion) :
return [get_complement(c) for c in dna[::-1]]

@@ -61,10 +73,25 @@ def rest_of_ORF(dna):
'ATG'
>>> rest_of_ORF("ATGAGATAGG")
'ATGAGA'
>>> rest_of_ORF("ATTTCGGGT")

Choose a reason for hiding this comment

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

Nice! Always add your own unit tests 👍


for a in all_orfs_two:
return_list.append(a)
#print(return_list)

Choose a reason for hiding this comment

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

please remove commented lines when submitting your final code

pass
orfs = find_all_ORFs_both_strands(dna)

#longest_size=len(max(orfs,key=len))

Choose a reason for hiding this comment

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

remove comment

if(leng > max_length):
max_length = leng

# maximum = max(lengths)

Choose a reason for hiding this comment

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

remove comment

@@ -130,8 +197,17 @@ def longest_ORF_noncoding(dna, num_trials):
dna: a DNA sequence
num_trials: the number of random shuffles
returns: the maximum length longest ORF """

Choose a reason for hiding this comment

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

One way to add unit tests for random functions is to use a fixed random seed. Check this documentation https://docs.python.org/3/library/random.html if this sounds interesting to you.

amino_sequences.append(coding_strand_to_AA(snip))

print(amino_sequences)
#print(len(amino_sequences))

Choose a reason for hiding this comment

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

remove comment

@author: Emma Westerhoff

"""

Choose a reason for hiding this comment

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

nice! I like the fact that you've learned the concept of dynamic programming. Something to think about : Now that you've learned about recursion, what would be the pros and cons of using recursion vs dynamic programming to tackle this problem?

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.

2 participants