-1

Its a function that checks whether a string is a Pangram or not so if str1 == 'the quick brown fox jumps over the lazy dog' the function will return True since the string contains every letter in the alphabet.

import string

def ispangram(str1, alphabet=string.ascii_lowercase):
    for char in set(alphabet): 
        if char in str1:
            return True
        else:
            return False
NAND
  • 663
  • 8
  • 22
Harry
  • 11
  • 3
  • Welcome to stackoverflow! To help us answer your question, please include more details, including what the expected output of your code is and what you're getting. – David Jun 09 '20 at 18:10
  • 1
    Isn't it just going to end the function (returning either `True/False`) after the first time it hits a `return` line? – BruceWayne Jun 09 '20 at 18:11
  • Flip the `if` and `return True` outside the loop, otherwise you're just returning True for the first character that is present in the input string. – Diptangsu Goswami Jun 09 '20 at 18:12
  • Does this answer your question? [How to check if string is a pangram?](https://stackoverflow.com/questions/24771381/how-to-check-if-string-is-a-pangram) – NAND Jun 09 '20 at 18:51

8 Answers8

3

As soon as a first letter is found in the str1, the function returns True.

What you need to do instead is:

def ispangram(str1, alphabet=string.ascii_lowercase):
    for char in set(alphabet): 
        if char not in str1:
            return False
    return True

This way the function returns False only if char is not in str1 and only after all the characters are found to be present in the str1 it returns True.

You might want to use str1.lower(), because you are checking for lowercase letters only.

IsawU
  • 430
  • 3
  • 12
  • Oh, that makes much more sense. Thank you! – Harry Jun 09 '20 at 18:21
  • What does `set` do for you in `for char in set(alphabet):`? Shouldn't it work just as well without converting `alphabet` to a `set`? – cdlane Jun 09 '20 at 18:38
  • `for char in alphabet:` will work just as well, but may not be as quick when a character is present multiple times in `alphabet`. Also, I'm editing the original code and merely pointing out the part causing the problem, without doing additional edits that may obfuscate the actual fix. – IsawU Jun 10 '20 at 07:10
1

Without knowing exactly what your function is supposed to do, I suspect the issue is that you return either True or False in the first iteration of the loop, so only the first letter is ever analyzed. Try this:

import string

def ispangram(str1, alphabet=string.ascii_lowercase):
    for char in set(alphabet): 
        if char not in str1:
            return False

    return True
DerekG
  • 3,555
  • 1
  • 11
  • 21
  • What does `set `do for you in `for char in set(alphabet):`? Shouldn't it work just as well without converting `alphabet` to a `set`? – cdlane Jun 09 '20 at 18:40
  • It would still work, the only thing `set()` accomplishes is that, in the case where there are duplicates in the alphabet, these duplicates need only be evaluated once, which could in some cases be more computationally expensive then generating the set – DerekG Jun 09 '20 at 18:43
1

Here is a different approach from yours:

import string

def ispangram(str1, alphabet=string.ascii_lowercase):
    return len(set(l.lower() for l in str1 if l.lower() in alphabet)) == len(alphabet)

print(ispangram("I am very tired, but I will keep programming to pass the quiz!"))

Output:

False
Red
  • 26,798
  • 7
  • 36
  • 58
  • 1
    You don't need the explicit returns of `True` and `False`, simply `return len(set(...)) == len(alphabet)`. The function doesn't need to know the answer, only the caller. – cdlane Jun 09 '20 at 18:37
1

You don't need to set(alphapet), you can iterate alphabet directly.

If you in the condition that function will return False then the function will exit without executing the following lines so after finishing the for-loop without returning false then we have pangram case so the function will directly return True.

import string

def ispangram(str1, alphabet=string.ascii_lowercase):
    for char in alphabet: 
        if char not in str1:
            return False         
    return True

str1 = 'the quick brown fox jumps over the lazy dog'
print(ispangram('Hello')) #False
print(ispangram(str1)) #True

However, we can shorten our function using all() to be

def ispangram(str1, alphabet=string.ascii_lowercase):
    return all([char in  str1 for char in alphabet])
NAND
  • 663
  • 8
  • 22
1

As noted elsewhere, you have your logic inverted and incorrectly indented. But your design is nice in that it naturally ignores characters that aren't letters. I would add some defense against mixed case and simplify it slightly:

from string import ascii_lowercase as alphabet

def ispangram(string):
    string = string.lower()

    for character in alphabet:
        if character not in string:
            return False

    return True


print(ispangram('Watch "Jeopardy!", Alex Trebek\'s fun TV quiz game.'))
cdlane
  • 40,441
  • 5
  • 32
  • 81
  • @AnnZen, first, reducing to a 'one liner' isn't the goal of all Python. Second, you blew it. By including the `[...]` brackets you made it a `list` comprehension that needs to complete before calling `all()` -- thus shortcutting on the first miss goes out the window so it is *less* efficient than my original. If you had left out the `[...]` brackets, then it would be a *generator expression* and would shortcut correctly, returning `False` on the first miss. – cdlane Jun 09 '20 at 18:48
  • Your function can be shortened to a liner: def ispangram(string): return all(character in string.lower() for character in alphabet) – Red Jun 09 '20 at 18:53
  • I agree, never said it was :) Still, I think using `all()` would be more pythonic. Thanks for the pointers about the generator. – Red Jun 09 '20 at 18:53
  • @AnnZen, your one liner calls `string.lower()` on *every* iteration when it only needs to be lower cased once. – cdlane Jun 09 '20 at 18:57
0

Your function returns True if any alphabet characters are found in your string. Is that what you wanted? It runs through the cars in set(alphabet), and if any of them are in your string if char in str1, it returns True. That doesn't seem very useful for your goal. You probably want more complicated logic, that checks the order of characters, whether the string is identical when reversed if you are looking at palindromes, etc. Hope this helps guide you!

Sam
  • 1,406
  • 1
  • 8
  • 11
0

Consider the input "abc". When your function iterates through the letters in the alphabet, it starts with "a", sees that it's in the input string, and then returns True. You need to check to make sure that all the letters from the alphabet are in the input string

David
  • 424
  • 3
  • 16
0

Your function returns directly after checking the first character of alphabet, regardless of whether the character in in str1. In fact, you function simply checks if some character of the alphabet is in you string.

What you intended to do was:

import string

def ispangram(str1, alphabet=string.ascii_lowercase):
    for char in set(alphabet): 
        if char not in str1:
            return False
    else:
        return True

However, a much simpler solution would simply be:

def ispangram(str1, alphabet=string.ascii_lowercase)
    return set(str1) >= set(alphabet)
Amitai Irron
  • 1,973
  • 1
  • 12
  • 14