1
\$\begingroup\$

I am looking for the way to speed up my script. The final output is as required, but it's clunky and really slow. I know it's slow because of the double for..in.. loops. So, how could I make it a bit smoother?

My script updates existing feature classes. I have two folders where all up-to-date shapefiles (shp) are stored. My script checks what shps are in each folder, and then searches for the same named featureclasses (these are older versions) in 4 geodatabases. So shp has to go to the gdb that already contains featureclass with the same name. I hope it is simple enough. Just to give you some idea, it takes over 1h to run this script, I have about 45 shapefiles (~1gb), these are copied over to 4 geodatabases.

Is there any way to avoid these time consuming double for..in.. loops? I need to mention that folders are stored on different server than gdb's, and that is a main reason of having it so slow. When I tested the script using only C drive it took 6-8minutes!

import time
import arcpy


startTime = time.clock()

print "Start"

FCProducts = r"C:\Python\DataConversion\shapefiles"
GBProducts = r"C:\Python\DataConversion\shapefile_gp"
adminBdry = r"C:\Python\DataConversion\temp1.gdb"
GnR = r"C:\DataConversion\temp2.gdb"
Invenories = r"C:\Python\DataConversion\temp3.gdb"
NFE = r"C:\Python\DataConversion\temp4.gdb"


arcpy.env.overwriteOutput = True

arcpy.env.workspace = FCProducts
listFC = [0]
listFC = arcpy.ListFeatureClasses()
del arcpy.env.workspace

for product in listFC:
    f2fOutput = product[:-4] + "_new"

    arcpy.env.workspace = GnR
    listGnrFC = arcpy.ListFeatureClasses()
    for gnrFC in listGnrFC:
        if gnrFC == product[:-4]:
            arcpy.FeatureClassToFeatureClass_conversion(product[:-4], GnR, f2fOutput)
            print product, " copied to ", GnR
            arcpy.Delete_management(gnrFC)
            print "old ", gnrFC, " deleted"
            arcpy.Rename_management(f2fOutput, product[:-4])
            print "output renamed", "\n"

    arcpy.env.workspace = adminBdry
    listAdBdryFC = arcpy.ListFeatureClasses()
    for AdBdry in listAdBdryFC:
        if AdBdry == product[:-4]:
            arcpy.FeatureClassToFeatureClass_conversion(product[:-4], adminBdry, f2fOutput)
            print product, " copied to ", adminBdry
            arcpy.Delete_management(AdBdry)
            print "old ", AdBdry, " deleted"
            arcpy.Rename_management(f2fOutput, product[:-4])
            print "output renamed", "\n"

    arcpy.env.workspace = Invenories
    listInventFC = arcpy.ListFeatureClasses()
    for invent in listInventFC:
        if invent == product[:-4]:
            arcpy.FeatureClassToFeatureClass_conversion(product[:-4], Invenories, f2fOutput)
            print product, " copied to ", Invenories
            arcpy.Delete_management(invent)
            print "old ", invent, " deleted"
            arcpy.Rename_management(f2fOutput, product[:-4])
            print "output renamed", "\n"

    arcpy.env.workspace = NFE
    listNfeFC = arcpy.ListFeatureClasses()
    for nfe in listNfeFC:
        if nfe == product[:-4]:
            arcpy.FeatureClassToFeatureClass_conversion(product[:-4], NFE, f2fOutput)
            print product, " copied to ", NFE
            arcpy.Delete_management(nfe)
            print "old ", nfe, " deleted"
            arcpy.Rename_management(f2fOutput, product[:-4])
            print "output renamed", "\n"

arcpy.env.workspace = GBProducts
listFCgb = [0]
listFCgb = arcpy.ListFeatureClasses()

for productGB in listFCgb:
    f2fOutputGB = productGB[:-4] + "_new"
    arcpy.env.workspace = Invenories
    listInventGB = arcpy.ListFeatureClasses()
    for InventGB in listInventGB:
        if InventGB == productGB[:-4]:
            arcpy.FeatureClassToFeatureClass_conversion(productGB[:-4], Invenories, f2fOutputGB)
            print productGB, " copied to ", Invenories
            arcpy.Delete_management(InventGB)
            print "old ", InventGB, " deleted"
            arcpy.Rename_management(f2fOutputGB, productGB[:-4])
            print "output renamed", "\n"

    arcpy.env.workspace = NFE
    listNfeGB = arcpy.ListFeatureClasses()
    for nfeGB in listNfeGB:
        if nfeGB == productGB[:-4]:
            arcpy.FeatureClassToFeatureClass_conversion(productGB[:-4], NFE, f2fOutputGB)
            print productGB, " copied to ", NFE
            arcpy.Delete_management(nfeGB)
            print "old ", nfeGB, " deleted"
            arcpy.Rename_management(f2fOutputGB, productGB[:-4])
            print "output renamed", "\n"
print "completed within:", time.clock() - startTime, "seconds", "\n"

I am using Windows XP, ArcGis 10.0, Python 2.6.6

\$\endgroup\$
  • 1
    \$\begingroup\$ Also is there any particular reason you're using Python 2.6 instead of the latest 2.7? \$\endgroup\$ – SuperBiasedMan Nov 20 '15 at 11:36
  • 1
    \$\begingroup\$ @SuperBiasedMan, I am using ArcGis 10.0 but script needs to be run on 9.x as well. Here is some guide what python can be used with which ArcGis support.esri.com/fr/knowledgebase/techarticles/detail/43889 \$\endgroup\$ – user3328469 Nov 20 '15 at 11:50
  • \$\begingroup\$ Yeah I assumed it was compatability. Just checking to make sure (and because no doubt someone else would have asked :P) \$\endgroup\$ – SuperBiasedMan Nov 20 '15 at 11:56
5
\$\begingroup\$

Don't repeat yourself

First, and of the least significance, Python spend more time building a substring rather than a local variable lookup. Thus you could improve thing a bit by naming product[:-4] at the start of your loop.

More importantly, the majority of your code is similar:

  • set a new workspace;
  • build a list of classes out of it;
  • for each of these classes:
    • convert the class;
    • delete the old one;
    • rename the new one.

The only things that change are: the name of the workspace and the name of the product involved. Just write a function to handle it once:

import arcpy


FCProducts = r"C:\Python\DataConversion\shapefiles"
GBProducts = r"C:\Python\DataConversion\shapefile_gp"
adminBdry = r"C:\Python\DataConversion\temp1.gdb"
GnR = r"C:\DataConversion\temp2.gdb"
Invenories = r"C:\Python\DataConversion\temp3.gdb"
NFE = r"C:\Python\DataConversion\temp4.gdb"

arcpy.env.overwriteOutput = True

def convert_class(workspace, product_name, output_name):
    arcpy.env.workspace = workspace
    for product in arcpy.ListFeatureClasses():
        if product == product_name:
            arcpy.FeatureClassToFeatureClass_conversion(product_name, workspace, output_name)
            print product_name, " copied to ", workspace
            arcpy.Delete_management(product)
            print "old ", product, " deleted"
            arcpy.Rename_management(output_name, product_name)
            print "output renamed", "\n"

arcpy.env.workspace = FCProducts
listFC = arcpy.ListFeatureClasses()

for product in listFC:
    f2fOutput = product[:-4] + "_new"
    product_name = product[:-4]
    convert_class(GnR, product_name, f2fOutput)
    convert_class(adminBdry, product_name, f2fOutput)
    convert_class(Invenories, product_name, f2fOutput)
    convert_class(NFE, product_name, f2fOutput)

arcpy.env.workspace = GBProducts
listFCgb = arcpy.ListFeatureClasses()

for productGB in listFCgb:
    f2fOutputGB = productGB[:-4] + "_new"
    product_name = productGB[:-4]
    convert_class(Invenories, product_name, f2fOutput)
    convert_class(NFE, product_name, f2fOutput)

Use resources sparingly

Now that the code is simpler to read, what exactly is it doing? For each item you are switching workspaces 2 or 4 times. Switching workspace meaning:

  • opening a file;
  • reading it;
  • parsing it;
  • building data structures out of it.

Doing that even once for each item is an enormous waste of time and computing resources. Better opening a workspace once, iterating over each product and then switching to an other workspace. The code layout for a single workspace would be something like:

def process_workspace(workspace, products):
    arcpy.env.workspace = workspace
    for product in products:
        product_name = product[:-4]
        output_name = product_name + "_new"
        for item in arcpy.ListFeatureClasses():
            if item == product_name:
                arcpy.FeatureClassToFeatureClass_conversion(product_name, workspace, output_name)
                print product, " copied to ", workspace
                arcpy.Delete_management(item)
                print "old ", item, " deleted"
                arcpy.Rename_management(output_name, product_name)
                print "output renamed\n"

You then just need to change workspace once before each of your *.gdb processing:

arcpy.env.workspace = FCProducts
listFC = arcpy.ListFeatureClasses()
process_workspace(GnR, listFC)
process_workspace(adminBdry, listFC)
process_workspace(Invenories, listFC)
process_workspace(NFE, listFC)

arcpy.env.workspace = GBProducts
listGB = arcpy.ListFeatureClasses()
process_workspace(Invenories, listGB)
process_workspace(NFE, listGB)

You could even improve the layout with for loop over the workspaces.

Putting it all together

By sticking a little bit more to PEP8 on line length and variable naming, we can end up with:

import arcpy

FC_PRODUCTS = r"C:\Python\DataConversion\shapefiles"
GB_PRODUCTS = r"C:\Python\DataConversion\shapefile_gp"

ADMIN_BOUNDARY = r"C:\Python\DataConversion\temp1.gdb"
GNR = r"C:\DataConversion\temp2.gdb"
INVENORIES = r"C:\Python\DataConversion\temp3.gdb"
NFE = r"C:\Python\DataConversion\temp4.gdb"


def process_workspace(workspace, products):
    arcpy.env.workspace = workspace
    for product in products:
        product_name = product[:-4]
        output_name = product_name + "_new"
        for item in arcpy.ListFeatureClasses():
            if item == product_name:
                arcpy.FeatureClassToFeatureClass_conversion(
                    product_name, workspace, output_name)
                print product, " copied to ", workspace
                arcpy.Delete_management(item)
                print "old ", item, " deleted"
                arcpy.Rename_management(output_name, product_name)
                print "output renamed\n"

arcpy.env.workspace = FC_PRODUCTS
products = arcpy.ListFeatureClasses()
for workspace in (GNR, ADMIN_BOUNDARY, INVENORIES, NFE):
    process_workspace(workspace, products)

arcpy.env.workspace = GB_PRODUCTS
products = arcpy.ListFeatureClasses()
for workspace in (INVENORIES, NFE):
    process_workspace(workspace, products)
\$\endgroup\$
  • \$\begingroup\$ thanks for your reply, I will definitely try to use your hints. You are totally right that I am repeating more or less the same piece of code, but only because scripting is not my environment, I am coming from GIS world  I will definitely give a try of your code. \$\endgroup\$ – user3328469 Nov 20 '15 at 14:43
  • \$\begingroup\$ your script works perfectly fine, it is slightly quicker (~30sec), than my original one. I have tested it only on my test environment (local C drive). I will test it on real data at some point next week. I need to admit that your script looks great, thanks. \$\endgroup\$ – user3328469 Nov 20 '15 at 15:05
6
\$\begingroup\$

Have you actually profiled your script to be sure that your time issue really is related to the for ... in loops? I'm pretty confident that your timing issue is more likely related to you doing this over network shares. You state yourself that if using local folders it runs in 6-8 minutes, but using network shares it takes hours.

Depending on how many changes your loops does, you might consider changing it so that all the network shares a copied once to local folders, run your script which potentially modifies the folders, and then copy back to the network share. This would reduce the amount of smaller read/writes across the network, which might be your issue.

Now onto some style and code issues:

  • Get better names – Please don't use abbrevations in variable names. It makes your code way harder to read and understand. What is the GnR, FCProducts or listAdBdryFC. I was close to not reviewing this, simply due to bad naming
  • Gather similar code blocks – It seems like all the second level for loops, actually does the same but with different files. These code blocks could be gathered into functions, and you could make a loop calling this function one by one
  • Update instead of convert, delete and rename? – I don't know arcpy, but after testing for where you're using your time (read: it's not the actual for loop of Python!), possibly optimising how you do the needed change. Possibly arcpy has better ways of directly looking up the one item you are looking for, instead of traversering the entire list of feature classes?
  • Break out when finished updating – If your for loops are reading through massive amounts of feature classes, and the one item you are updating is located early in the list, you could benefit from doing a break after updating, so that you don't read through the rest of the list.
  • Debug-tip: Add time to print's – If you're unsure where your time vanishes you could either run profilers, or you could add current time to the start of your generours output. You don't need to add the deltas, as it is easy to detect time spans when just looking down the time presented at start of line.

Here is your code refactore for style and simplification issues:

from __future__ import print_functions
import time
import arcpy

FC_PRODUCTS = r"C:\Python\DataConversion\shapefiles"
GB_PRODUCTS = r"C:\Python\DataConversion\shapefile_gp"
ADMIN_BDRY = r"C:\Python\DataConversion\temp1.gdb"
G_N_R = r"C:\DataConversion\temp2.gdb"
INVENORIES = r"C:\Python\DataConversion\temp3.gdb"
NFE = r"C:\Python\DataConversion\temp4.gdb"


def handle_file(filename, product):
    """Look through filename, and handle product_update."""

    new_product = product[:-4] + "_new"
    stripped_product = product[:-4]

    arcpy.env.workspace = filename
    feature_classes = arcpy.ListFeatureClasses()

    for feature_class in feature_classes:
        if feature_class == stripped_product:

            arcpy.FeatureClassToFeatureClass_conversion(stripped_product, filename, new_product)
            print('{}: {} copied to {}'.format(time.clock(), product, filename))

            arcpy.Delete_management(filename)
            print('{}: Old {} deleted'.format(time.clock(), filename))

            arcpy.Rename_management(new_product, stripped_product)
            print('{}: Output renamed\n'.format(time.clock()))

            return  # Done updating for this file (assuming only update pr. file)


def main():

    start_time = time.clock()
    print("{}: Start main".format(start_time))

    arcpy.env.overwriteOutput = True

    arcpy.env.workspace = FC_PRODUCTS
    listFC = [0]
    listFC = arcpy.ListFeatureClasses()
    del arcpy.env.workspace

    for product in listFC:
        for filename in (G_N_R, ADMIN_BDRY, INVENORIES, NFE):
            handle_file(filename, product)



    arcpy.env.workspace = GB_PRODUCTS
    listFCgb = [0]
    listFCgb = arcpy.ListFeatureClasses()

    for productGB in listFCgb:

        for filename in (INVENORIES, NFE):
            handle_file(filename, productGB)

    end_time = time.clock()
    print("{}: Ended within {} seconds".format(end_time, end_time - start_time))


if __name__ == '__main__':
    main()

When refactoring I also saw one possible major issue: For each product in either the FC_PRODUCTS or GB_PRODUCTS you read through each of the corresponding files updating one item at a time, possibly rewriting the file each time as well. This update converts, deletes and renames that particular item. This means that you have a time complexity of \$O(n*m)\$, where \$n\$ is items in the products file, and \$m\$ is items in the secondary file. This is bad...

You should aim for doing a single run through each of the secondary files reversing the lookup you now do. That is instead of looking through all products one at a time in the corresponding files, go through the corresponding files and see if it exists in the main file. This will change time complexity to \$O(m)\$ for each of the corresponding files. If you in addition can do in-memory handling (or delay writing of the file), you'll might save quite a lot of time!

Gist of code (still untested as I don't have data files or arcpy):

from __future__ import print_functions
import time
import arcpy

FC_PRODUCTS = r"C:\Python\DataConversion\shapefiles"
GB_PRODUCTS = r"C:\Python\DataConversion\shapefile_gp"
ADMIN_BDRY = r"C:\Python\DataConversion\temp1.gdb"
G_N_R = r"C:\DataConversion\temp2.gdb"
INVENORIES = r"C:\Python\DataConversion\temp3.gdb"
NFE = r"C:\Python\DataConversion\temp4.gdb"


def handle_file(main_features, filename):
    """Look through filename, and handle product_update."""

    arcpy.env.workspace = filename
    feature_classes = arcpy.ListFeatureClasses()

    for feature_class in feature_classes:
        if feature_class in main_features:

            # Get product from main_feature
            main_idx = main_features.index(feature_class)
            product = main_features[main_idx]
            new_product = product[:-4] + "_new"
            stripped_product = product[:-4]

            # Convert
            arcpy.FeatureClassToFeatureClass_conversion(stripped_product, filename, new_product)
            print('{}: {} copied to {}'.format(time.clock(), product, filename))

            # Delete
            arcpy.Delete_management(filename)
            print('{}: Old {} deleted'.format(time.clock(), filename))

            # Rename
            arcpy.Rename_management(new_product, stripped_product)
            print('{}: Output renamed\n'.format(time.clock()))


def handle_main_file(main_filename, *filenames):
    """Update main features in all the filenames."""

    # Clear it out for next run?
    del arcpy.env.workspace     

    arcpy.env.workspace = main_filename
    # main_features = [0]  # Why this line?
    main_features = set(arcpy.ListFeatureClasses())

    for filename in filenames:
        handle_file(main_features, filename)


def main():

    start_time = time.clock()
    print("{}: Start main".format(start_time))

    arcpy.env.overwriteOutput = True

    handle_main_file(FC_PRODUCTS, G_N_R, ADMIN_BDRY, INVENORIES, NFE) 
    handle_main_file(GB_PRODUCTS, INVENORIES, NFE)

    end_time = time.clock()
    print("{}: Ended within {} seconds".format(end_time, end_time - start_time))


if __name__ == '__main__':
    main()

Edit: As suggested by Joe Wallis, I've changed the main_features into a set to speed lookups. Do note, that this (as the rest) is still untested, but Joe Wallis should be correct that this specific lookup should then go a lot faster. And Joe Wallis is also correct in his answer that printing does take time, so you need to consider whether it is worthwhile to actually print inbetween each operation. I think I would keep it for a little while as it clearly indicates where you spend your time, and compared to read/writing files the time to print should be neglible.

\$\endgroup\$
  • \$\begingroup\$ as you probably noticed I am not a developer by any mean, That is one first codes, hence the level of question. Thanks for suggestion, this will take ma a while to adapt to my needs, but you gave me a good starting point. Answering your questions/style comments: better names - these are common abbreviations in my company, I know it is hard to read for some, but I didn’t want to use full names eg. Grants and Regulations (GnR), Administrative Boundary Forestry Commission (AdminBdryFC) etc. Gather similar block -that is why I asked my question, I knew it is possible but didn’t know how \$\endgroup\$ – user3328469 Nov 20 '15 at 14:31
  • \$\begingroup\$ Update instead… - Just like you have probably noticed I have used arcpy.env.OverwriteOutput ¬ and it is rather known issue that this is not working properly for some reason ( I have never found reasonable explanation for that) If I am not adding “_new” to the name, new feature class will be renamed automatically to “name_1” ( “_1” is added). That is why I want to rename it my self. Beak out - that is really useful hint, I will try to implement it. \$\endgroup\$ – user3328469 Nov 20 '15 at 14:32
  • \$\begingroup\$ Add time to prints – I have added all of these prints because the script may not be run only by me, it is kind of ‘… in progress’ marker, just to assure an user that script is still working as this may take over an hour to finish \$\endgroup\$ – user3328469 Nov 20 '15 at 14:32

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.