4
\$\begingroup\$

I am attempting to find folders based on their last modified dates.

import os, shutil, stat, errno, time, datetime
srcFolders = "C:\Users\Test\Desktop\Test"
archiveDate = datetime.datetime.strptime("2016/11/20", '%Y/%m/%d')
os.chdir(srcFolders)

for name in os.listdir('.'):
    if os.path.isdir(name):
        modifiedDate = time.strftime('%Y/%m/%d', time.gmtime(os.path.getmtime(name)))
        strLastModified = datetime.datetime.strptime(modifiedDate, '%Y/%m/%d')
        if  strLastModified > archiveDate:
            print name

This is what I've got and it seems to be right in that they're comparing the same attributes. If there is a better, more Pythonic way for this, please advise.

\$\endgroup\$
4
\$\begingroup\$
  • Do not put all imports on the same line and use snake_case rather than camelCase for variable names.

    Read PEP8, the official Python style guide to make your code look like Python code.

  • Use functions for better reusability. Instead of using hardcoded values like your folder path or archive date, use parameters. You will more easily be able to test various values in the interactive interpreter.

    This also means using the if __name__ == '__main__' construct.

  • return values instead of printing them in the function. Again, better reusability.

    For starter you can build a list and return it.

  • when using datetime objects, you can compare them directly. It is cleaner and probably faster than comparing their string representation. Building dates is also as simple as using the right constructor.

  • you don't need to move to the desired directory before listing its content. os.listdir can take the absolute path and work from that.

Revised code

import os
import datetime


def filter_by_date(src_folder, archive_date):
    relevant_folders = []
    for name in os.listdir(src_folder):
        full_name = os.path.join(src_folder, name)
        if os.path.isdir(full_name):
            if datetime.fromtimestamp(os.path.getmtime(full_name)) > archive_date:
                 relevant_folders.append(name)
    return relevant_folders


if __name__ == '__main__':
    print filter_by_date("C:\Users\Test\Desktop\Folder", datetime.datetime(2016, 11, 10))

Is a first aproach. But using append in a for loop is deemed unPythonic, better use a generator or a list-comprehension here:

import os
import datetime


def filter_by_date(src_folder, archive_date):
    os.chdir(src_folder)
    return [
        name for name in os.listdir('.')
        if os.path.isdir(name)
        and datetime.fromtimestamp(os.path.getmtime(name)) > archive_date
    ]


if __name__ == '__main__':
    print filter_by_date("C:\Users\Test\Desktop\Folder", datetime.datetime(2016, 11, 10))
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for this I did is {chdir} because when i was using {isdir} they would always return as not a directory. \$\endgroup\$ – CFNZ_Techie Dec 6 '16 at 19:35
  • \$\begingroup\$ @CFNZ_Techie Oh, yes, sorry for that. See updated code for an alternative. \$\endgroup\$ – 301_Moved_Permanently Dec 6 '16 at 19:43
  • \$\begingroup\$ I also seem to have an issue with ".fromtimestamp" it doesn't accept it. \$\endgroup\$ – CFNZ_Techie Dec 7 '16 at 2:56
  • 1
    \$\begingroup\$ @CFNZ_Techie Oh, I get what the error is now, os.path.getmtime(name) was called on a file that doesn't exist. Using os.path.join or os.chdir should solve the issue. See updated answer. \$\endgroup\$ – 301_Moved_Permanently Dec 7 '16 at 19:34
  • 1
    \$\begingroup\$ @CFNZ_Techie oh yes, or you could from datetime import datetime and remove one datetime from the last line. \$\endgroup\$ – 301_Moved_Permanently Dec 7 '16 at 20:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.