8000 Implementing protein-translation by Average-user · Pull Request #630 · exercism/haskell · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Implementing protein-translation #630

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Feb 14, 2018
Merged

Implementing protein-translation #630

merged 6 commits into from
Feb 14, 2018

Conversation

Average-user
Copy link
Member

I wanted to use chunksOf from Data.List.Split but apparently it is not in the cabal configuration

@Average-user
Copy link
Member Author

@petertseng The Travis errors are all because the README ¿aren't they?

@petertseng
Copy link
Member

I will review when I am able. I am sorry to say that I am currently unable.

@Average-user
Copy link
Member Author
Average-user commented Dec 12, 2017 •
edited
Loading

@petertseng Don't worry, I have no rush. Too much AoC?

petertseng
petertseng suggested changes Jan 10, 2018
View reviewed changes
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.

The tests seems reasonable and the implementation is reasonable, so I think I only ask to change the placement and some versions.

There is the natural question of: Should either the nucleotides ACGU or the codons (Methionine, Tryptophan, etc) be made into a data, but I don't think it's needed, and I think I interpret #626 (comment) to agree with that (I hope I did not interpret)

exercises/protein-translation/examples/success-standard/src/ProteinTranslation.hs Outdated

proteins :: String -> Maybe [String]
proteins xs = if length xs `mod` 3 == 0 && all validCodon codons
then Just (takeWhile (/= "STOP") (map proteins' codons))
Copy link
Member

Choose a reason for hiding this comment

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

I think the extra space after then could go away

exercises/protein-translation/package.yaml
@@ -0,0 +1,20 @@
name: protein-translation
version: 1.1.0.1
Copy link
Member
@petertseng petertseng Jan 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

1.0.1.1 because https://github.com/exercism/problem-specifications/blob/master/exercises/protein-translation/canonical-data.json#L3 says 1.0.1 (unless it changes after I wrote this comment)

It changed, so 1.1.0.1 is correct.

exercises/protein-translation/stack.yaml Outdated
@@ -0,0 +1 @@
resolver: lts-9.11
Copy link
Member

Choose a reason for hiding this comment

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

I moved to lts-10.2

exercises/protein-translation/README.md Outdated
@@ -0,0 +1,3 @@
```Haskell
f = error "regenerate me!""
Copy link
Member

Choose a reason for hiding this comment

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

I remind myself

config.json
@@ -13,6 +13,16 @@
"topics": [
]
},
{
"uuid": "f750e380-fe16-42c0-8b3d-8acd7e443a6e",
"slug": "protein-translation",
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't put this between hello-world and leap because leap seems easier since don't have to deal with lists or Maybe. So at the very least, after leap.

@Average-user
Copy link
Member Author

And what about Data.List.Split? How can I do to make use of it?

@petertseng
Copy link
Member

You are optionally free to do so if you want to, but it is not required. If you want ideas, you can see how other exercises use it.

$ git grep Split                                 (01-11 10:28)
exercises/atbash-cipher/examples/success-standard/src/Atbash.hs:import Data.List.Split (chunksOf)
exercises/crypto-square/examples/success-standard/src/CryptoSquare.hs:import Data.List.Split (chunksOf)
exercises/kindergarten-garden/examples/success-standard/src/Garden.hs:import Data.List.Split (chunksOf)
exercises/ocr-numbers/examples/success-standard/src/OCR.hs:import Data.List.Split (chunksOf)
exercises/word-count/examples/success-simple/src/WordCount.hs:import Data.List.Split (wordsBy)

@Average-user
Copy link
Member Author

I know how to use it, but the build complains about it, dont know why
8000

petertseng
petertseng approved these changes Feb 14, 2018
View reviewed changes
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.

should be good now

made it difficulty 3

@petertseng petertseng merged commit ac0df8f into exercism:master Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petertseng petertseng petertseng approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Average-user @petertseng
0