Your first block, reading the csv can be changed in two ways.
You change EmailDict = dict()
to email_dict = {}
.
It's uncommon to do dict()
unless there is a requirement, like type conversion.
Also Python uses snake_case
for variables.
You can change the block to just use a dictionary comprehension.
It's simple to setup and voids my above comment.
All it is is syntatic suger over what you have to increas readability.
with open('commissionrepemaillist.csv', 'r') as infile:
reader = csv.reader(infile)
email_dict = {
row[0]: row[1], row[2]
for row in reader
}
While it removes the information about what the different items are,
it makes reading much faster. And you code to look nicer.
You can use another dictionary comprehension to generate file_dict
.
I personally have never come across re.split("[_.]", FILE)
, but it's an neet idea!
file_dict = {
re.split("[_.]", file_name)[1]: file_name
for file_name in os.listdir(path)
}
You normally want to loop through the smaller dictionary.
I assume that the folder has more, if not lots more, files than you wish to send in the email.
This is as the check if ID in EmailDict:
is \$O(1)\$ not \$O(e)\$, where as for ID in FileDict
is \$O(f)\$.
If we get the time it takes to loop through either it will be \$O(e \times 1)\$ for EmailDict
,
and \$O(f \times 1)\$ for FileDict
. And so you want to pick the smaller of the two.
However, it's unlikely that it will have too much of an impact, and so you may ignore this if you wish.
You can use dict.iteritems()
to iterate through both the ID and the file name.
There is a subtle difference between dict.items
and dict.iteritems
,
the latter is in most cases better, and so became the default in Python3.
It's usage is:
for ID, file_name in file_dict.iteritems():
if ID in email_dict:
...
You redefine path
in each enumeration to the same static path.
This hinders execution time a little, and is not productive.
Also file
is python keyword, and so it's highly advised against re-assigning it.
This is as from that point on, as you redefined it in the global scope,
if you wish to use it you will have to use __builtin__.file
, which is definitely not good. I would use file_path
instead.
The rest of the block, apart from else: continue
, seems good.
Last block now.
There seems to be a bug(?) or an intention to send all files, even un-encrypted ones,
to the last person in the csv...
EMAIL
is only defined in the first block, or with my changes not at all.
And due to the lack of check for if ID in email_dict
,
it means that even un-encrypted files will be sent,
as only files with the ID in both email_dict
and file_dict
will be encrypted.
Other than the same problems that were highlighted in the other blocks.
I would assume that you don't have to create a new smtp connection for each email.
Instead I would move the all bar s.sendmail(...)
out of the loop.
Higher level stuff:
I would do both the encryption and the email in the same loop.
However I would encourage the use of a function, to make the logic easier to read.
Use 'raw raw string'. A raw string in Python is "Hello world"
,
a 'raw raw string' has an r in-front of it, and prevents character escaping.
For example r'\n'
is '\\n'
.
Stop over-writing path, you're making more work for when you change the path!
Or want to use this script in a different place.
I think over-writing the files is a bad idea, instead I would make a new file,
which would be something like file_name + '_cypt'
.
It's generally better to use str.format
over %
. This is as %
has quirks.
As this is a real world application, you would want to only execute if it's the main program.
This is just a small security feature.
if __name__ == '__main__':
...
And so I would re-write your program to the following:
import os
import re
import csv
import PyPDF2
import smtplib
from email.mime.text import MIMEText
from email.mime.application import MIMEApplication
from email.mime.multipart import MIMEMultipart
PATH = r"C:\Apps\CorVu\DATA\Reports\Monthly Commission Reports\Output\pdcom1"
def encrypt(file_path, password):
input_pdf = PyPDF2.PdfFileReader(file_path)
output = PyPDF2.PdfFileWriter()
output.appendPagesFromReader(inputStream)
output.encrypt(password)
with open(file, 'wb') as outputStream:
# Is this a bug? Should it not be the other way around?
output.write(outputStream)
def send_email(file_path, email, ID):
with open(file_path, 'rb') as pdf :
pdf = MIMEApplication(pdf.read(), _subtype = 'pdf')
pdf.add_header(
'content-disposition',
'attachment',
filename=('MonthlyPaidCommission_{}.pdf'.format(ID)))
text = MIMEText(
"""For Rep Number : {}
Please use the password previously emailed to open attached file.
If you have any questions or need assistance please contact me.
Thank you""".format(ID))
msg = MIMEMultipart(_subparts=(text, pdf))
msg['SUBJECT'] = "Commission Report"
msg['FROM'] = "me@gmail.com"
msg['TO'] = email
s = smtplib.SMTP("LOCALHOST:587")
s.starttls()
s.login('user', 'pwd')
s.sendmail('me@123.com', email, msg.as_string())
s.quit()
if __name__ == '__main__':
with open('commissionrepemaillist.csv', 'r') as infile:
reader = csv.reader(infile)
emails = {
row[0]: row[1], row[2]
for row in reader
}
files = {
re.split('[_.]', file_name)[1]
for file_name in os.listdir(path)
}
for ID, email_info in emails:
if ID in files:
file_path = os.path.join(PATH + files[ID])
encrypt(file_path, email_info[1])
send_email(file_path, email_info[0], ID)