Skip to content

Isogram: Exercise for isogram with test cases. #367

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

Closed
wants to merge 1 commit into from

Conversation

sbulage
Copy link

@sbulage sbulage commented Oct 4, 2016

Written python code which checks whether given string is isogram or not.

@sbulage sbulage changed the title Written code for isogram in python with test case. Code for isogram in python with test case. Oct 4, 2016
@sbulage
Copy link
Author

sbulage commented Oct 4, 2016

@kytrinyx Kindly review my code.

@sbulage sbulage changed the title Code for isogram in python with test case. Isogram: Exercise for isogram with test cases. Oct 4, 2016
@kytrinyx
Copy link
Member

kytrinyx commented Oct 4, 2016

@sbulage thanks for the contribution!

The build is failing with the following information in the log:

$ ./bin/configlet .
Evaluating .
-> config.json does not include [isogram].

The command "./bin/configlet ." exited with 1.

Configlet is an exercism-specific linter that checks some of the basic things about an exercise. The way to fix this is to add "isogram" to the "exercises" key in the config.json file: https://github.com/exercism/xpython/blob/master/config.json#L69

You can make the "difficulty" default to 1 and the "topics" an empty array. We will be filling in this information later.

The "exercises" array is ordered roughly by difficulty of the exercise, so "isogram" should probably be not too early, not too late. Maybe in the second quarter of exercises somewhere. It doesn't matter too much getting it exactly right, as we can adjust this later when we get some feedback from people who solve the exercise.

An isogram (also known as a "nonpattern word") is a word or phrase without a repeating letter.

Examples of isograms:

    -lumberjacks
    -background
    -downstream
The word isograms, however, is not an isogram, because the s repeats.
@sbulage
Copy link
Author

sbulage commented Oct 5, 2016

@kytrinyx Thanks for your suggestions. I made the required changes.

@arcuru
Copy link
Contributor

arcuru commented Oct 7, 2016

For adding isogram into the exercises array, I suggest placing it before pangram as they are very similar problems, and this actually serves as a very good warmup to that problem.

In addition, please add the rest of the common test cases from the x-common repo. https://github.com/exercism/x-common/blob/master/exercises/isogram/canonical-data.json

The tests should all be broken up so that they test one word per test case. Look at how the other problems do it.

@sbulage
Copy link
Author

sbulage commented Oct 12, 2016

@patricksjackson Sure, I will write the test cases from x-common. 👍

@behrtam
Copy link
Contributor

behrtam commented Oct 19, 2016

@sbulage Do you need any help completing this exercise? I'm happy to help.

@sbulage
Copy link
Author

sbulage commented Oct 22, 2016

@behrtam No, Thanks :)
There are some test cases which are returning isogram_test as False but below link it is showing as True as it contains some whitespaces.

Link --> https://github.com/exercism/x-common/blob/master/exercises/isogram/canonical-data.json

@behrtam
Copy link
Contributor

behrtam commented Oct 22, 2016

A)
Looks like white spaces don't count. So you need to either remove them first or find a way to ignore them.

B)

from example import isogram

Than you need to fix your import. I would suggest to rename your function.

from isogram import is_isogram

You could keep both files (example.py and isogram.py) or use the test script (./test/check-exercises.py) which will create a temporary file from the example one.

C)
Finally you need to break up the different test cases and add the missing ones as @patricksjackson already pointed out. Than we are good to go and can merge this new exercise.

@behrtam
Copy link
Contributor

behrtam commented Feb 21, 2017

Any chance that you might find some time to finish this PR?

@behrtam behrtam mentioned this pull request Mar 7, 2017
@behrtam behrtam closed this Mar 19, 2017
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.

4 participants