-
-
Notifications
You must be signed in to change notification settings - Fork 68
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
Added protein translation practice exercise #693
base: main
Are you sure you want to change the base?
Conversation
Hi, thanks for your interest. Please rebase and make the tests pass, then ping reviewers again. |
Hi @cmcaine , |
Julia 1.6 is the current LTS release, so I think we should still support it. The bug is probably because the macros are being expanded at different times on different releases, which might have been done deliberately or not. We could fix that with eval ( How does that sound to you? |
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.
as noted above
I made the changes, but then the string macro is a bit redundant, isn't it? |
@@ -0,0 +1,51 @@ | |||
codon_protein_dict = Dict( |
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.
codon_protein_dict = Dict( | |
const codon_protein_dict = Dict( |
macro rna_str() | ||
# I'm a ribosome macro! | ||
end | ||
|
||
|
||
function rna_translator() | ||
# I'm a ribosome function! | ||
end |
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.
Reordering these to encourage the student to work on the function first.
Also adding a link to the docs because macros are confusing.
macro rna_str() | |
# I'm a ribosome macro! | |
end | |
function rna_translator() | |
# I'm a ribosome function! | |
end | |
function rna_translator() | |
# I'm a ribosome function! | |
end | |
# Read the manual for more information on string macros: | |
# https://docs.julialang.org/en/v1/manual/metaprogramming/#meta-non-standard-string-literals | |
macro rna_str() | |
# I'm a ribosome macro! | |
end |
The macro serves the same purpose as other string macros:
What do you want the student to learn from the exercise? I suggest changing all instances of rna"..." in the tests to rna_translator("...") and adding a couple of tests of the macro at the end. Reason: the function interface is easier to test and understand for the student. Alternatively, we could remove the macro version entirely. |
I wanted to teach both macro writing and creating an exception. I'll settle for only learning how to create a new exception. |
@@ -0,0 +1,3 @@ | |||
function rna_translator(rna::String) |
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.
Function names should describe an action, not an object, according to the Julia style guide and general practice.
How about one of these names? Presented in my preference order.
function rna_translator(rna::String) | |
function rna_to_amino_acids(rna::String) |
function rna_translator(rna::String) | |
function rna_to_protein(rna::String) |
function rna_translator(rna::String) | |
function translate_rna(rna::String) |
Feel free to put the string macro back in as a bonus question, if you like. The rot13 practice exercise has an example of this. |
Renamed and added the macro as a bonus question |
Can we merge this? |
Sorry, I missed the updates. I'll try to take a look this week. Please ping me again if I don't. |
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.
Note to self. Last two commiys look good. Like the rename. Recheck the diff with master before merge.
ping @cmcaine |
@cmcaine Can we merge? |
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.
Thanks for the reminder.
What do you think of these suggestions?
exercises/practice/protein-translation/.docs/instructions.append.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Colin Caine <cmcaine@gmail.com>
Co-authored-by: Colin Caine <cmcaine@gmail.com>
I comitted the suggested changes |
Cool. The example.jl will need to be changed to throw Argument error. |
Here is a suggestion for the protein-translation exercise in Julia using string macros. Let me know what you think.