Skip to main content
2 of 3
added 857 characters in body
janos
  • 113.1k
  • 15
  • 154
  • 396

PEP8

You're not following PEP8, Python's official coding style guide. A couple of violations that jump in the eye:

  • Don't put multiple imports on the same line
  • Put a space after commas in parameter lists, so m.login(user, pwd) instead of m.login(user,pwd)
  • There shouldn't be whitespace after :
  • Use at least 2 spaces before inline comments (before each # comment)
  • Put a space after # when starting comments, so # comment instead of #comment
  • Some lines are too long (longer than 79 characters)

Bad practices

  • Remove unused imports: getpass, csv, threading
  • No need for the parens in user = ("username"), and it may get confused as a tuple (which it isn't)
  • The variable m is poorly named: judging by the name I might guess it's a "message", when it's actually a mailbox
  • At the point where you do str(message), message might not have been assigned yet
  • At the point where you do os.listdir(path), path might not have been assigned yet

In this code:

resp, items = m.search(None, "ALL")
items = items[0].split()  # getting the mails id

for emailid in items:
    resp, data = m.fetch(emailid, "(RFC822)")

The initialization of resp is pointless, because it will be reassigned anyway in the loop. Effectively you throw away the first assignment of resp. In such cases a common practice is to use _ as the variable name, like this:

_, items = m.search(None, "ALL")

On closer look, I see that after the 2nd assignment too, resp is not used again. So it could be replaced with _ there too.


message = re.compile(r'\%(.+?)\%', re.DOTALL).findall(content)

This is the same as the above but simpler:

message = re.findall(r'\%(.+?)\%', content, re.DOTALL)

But since you have this code inside a for loop, it would be best to compile the pattern once before the loop, and reuse it inside, like this:

re_between_percent = re.compile(r'\%(.+?)\%', re.DOTALL)
for part in mail.walk():
    # ...
    message = re_between_percent.findall(content)

Do the same for the other statements too where you do re.compile(...).findall(...).


Instead of this:

    htmlData = open(os.path.join('directory', htmlFile), 'w+')
    htmlData.write(htmlCode)
    print(htmlFile + ' Complete')
    htmlData.close()

The Pythonic way to work with files:

    with open(os.path.join('directory', htmlFile), 'w+') as htmlData:
        htmlData.write(htmlCode)
        print(htmlFile + ' Complete')
janos
  • 113.1k
  • 15
  • 154
  • 396