Return to Revisions

Remarks:

• Bug: you have an extra letter sneaking in with for req in range(0, nr_letters + 1):, making your output longer than it should be. That should be for req in range(0, nr_letters): or for req in range(nr_letters):. The other calls like for sym in range(1, nr_symbols + 1): can be simplified as well. Try to eliminate these little + and - operations. You generally don't need them, and off by one errors are one of the classic subtle programming bugs.
• In the line above, req is unused, so replace it with a _, by convention.
• Avoid indexing--prefer random.choices to select random items from a list (random.choice if you just want to select one item).
  • Tiny bug: random.randint(0,len(numbers)) should be random.randint(0, len(numbers) - 1) since the end is inclusive, and don't subtract 1 once you have this index. You're subtracting 1 later, so your range is -1 to len(numbers) - 1. The -1 is a valid index in Python so your code won't raise an IndexError, but it means there'll be a bit of extra weight added to the last item.
• Separate user interaction (I/O) and algorithm logic.
• Use a function to make your code reusable, testable and abstract away complexity.
• Add type hints to your function and check them with pyright.
• Don't use f-strings unless there's actually interpolation happening.
• Don't assume input is valid; handle errors gracefully.
• There is no performance issue with your code, so "optimization" doesn't matter at this point. Focus on coding style and writing maintainable code.
• Add docstrings.
• Consider adding unit tests. Since your code uses randomness, you can use a regex or logic to make sure the password has the right stuff in it, or seed the random library to be deterministic. But even writing a simple length test would have caught your primary bug mentioned above. Fuzz testing can also be used to catch edge case crashes and make sure your input sanitization and exception handling is tight. A comprehensive test suite means you can refactor with a lower likelihood of causing regressions.

Here's a rewrite:

from random import choices, shuffle
from string import ascii_letters, digits


def generate_password(nr_letters: int, nr_numbers: int, nr_symbols: int) -> str:
    """Generates a password with n random letters, numbers and symbols"""
    symbols = "!#$%&()*+"
    password = [
        *choices(ascii_letters, k=nr_letters),
        *choices(digits, k=nr_numbers),
        *choices(symbols, k=nr_symbols),
    ]
    shuffle(password)
    return "".join(password)


def read_positive_int(
    prompt: str, invalid: str = "Input must be a positive integer"
) -> int:
    """Reads a positive integer input from stdin, repeating until successful"""
    while True:
        try:
            if (n := int(input(prompt))) > 0:
                return n
        except ValueError:
            pass
        print(invalid)


def main():
    """Interacts with the user to generate a password"""
    print("Welcome to the PyPassword Generator!")
    nr_letters = read_positive_int("How many letters would you like in your password? ")
    nr_symbols = read_positive_int("How many symbols would you like? ")
    nr_numbers = read_positive_int("How many numbers would you like? ")
    password = generate_password(nr_letters, nr_numbers, nr_symbols)
    print(password)


if __name__ == "__main__":
    main()

You could generalize this a step further and accept arbitrary iterables and counts instead of hardcoded letters, numbers and symbols, but that seems a bit premature, so I left the function as-is in that respect.

3 positional args is getting to be a bit much, so you might want to make those kwargs as well, also left as a future improvement.

I'm not a security expert, so I can't remark on whether this should be used for actual passwords. Erring on the side of caution, I'd say don't use it in favor of an expert-vetted algorithm. See Typical password generator in Python for our community thread for discussion.