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

Protein translation (fix #267) #305

Merged
merged 3 commits into from
Jun 18, 2017

Conversation

go717franciswang
Copy link
Contributor

Split the earlier pull request into 2 commits -- remove tests for codon compression, and add tests for of_rna. It resolves #267.

* README didn't mention about compressed codons. So, test cases involing
  compressed codons have been removed or modified.
* Example has been simplified because it no longer require codon
  'decompression'.
* Added test cases for function `of_rna`, which translates a strand of
  RNA into a sequence of proteins per README. The test cases are copied
  from the test cases in
  [xpython](https://github.com/exercism/xpython/blob/master/exercises/protein-translation/protein_translation_test.py).
* Added example for `of_rna`.
@petertseng
Copy link
Member

Nice, looks like it's more understandable as two commits.

An interesting thing happened. The https://github.com/exercism/x-common/blob/master/exercises/protein-translation/description.md is talking about RNA (ACGU), but nucleotide-codons (https://github.com/exercism/x-common/blob/master/exercises/nucleotide-codons/description.md) used to talk about DNA (ACGT) before #258 blindly renamed nucleotide-codons to protein-translation.

It would probably be for the best if all the T were changed to U, that's my fault. You can do it if you like otherwise I will have to soon.

@go717franciswang
Copy link
Contributor Author

Ah. I see. I referenced wikipedia, and switched the DNA codons with RNA codons in a new commit.

Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

OK, everything seems to make sense here. Was removal of CGA intentional?

}

#[test]
#[ignore]
fn test_arginine_name() {
let info = proteins::parse(make_pairs());
assert_eq!(info.name_for("CGA"), Ok("arginine"));
Copy link
Member

Choose a reason for hiding this comment

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

removal intentional? I notice CGA is still an arginine in https://en.wikipedia.org/wiki/Genetic_code#RNA_codon_table

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad. That was a mistake. Let me add the missing ones.

Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

Very good, if nobody has anything else to say by the end of the weekend let's merge.

I would suggest to squash the last two commits together ("Replace DNA codons with RNA codons" + "Add some missing codons"). Feel free to do that, otherwise I will before merging.

Add some missing codons
@go717franciswang
Copy link
Contributor Author

Done. The last 2 commits were squashed.

@petertseng petertseng merged commit 772d684 into exercism:master Jun 18, 2017
@petertseng
Copy link
Member

All right. Thanks for the work!

@go717franciswang go717franciswang deleted the protein-translation branch June 19, 2017 01:55
@go717franciswang
Copy link
Contributor Author

Awesome. Thanks!

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.

Tests for protein-translation don't match its README
3 participants