4
\$\begingroup\$

I have written this script that downloads two tables from the db, preforms an intersection on them and adds 2 new columns to the resulting table, and uploads the resulting table to the db. It's a bit slow and I realize my code is messy (I'm new to Python and GIS). I suspect more steps can be done in parallel, for example downloading both tables from the database at once.

import arcpy, os, subprocess
import logging, shutil, sys


if __name__ == '__main__':
    #create logger
    logger = logging.getLogger(__name__)
    FORMAT = '%(name)s - %(levelname)s - %(message)s'
    logging.basicConfig(filename='py_intersection.log', level=logging.DEBUG, format=FORMAT)
    logger.info('Started shp intr arcpy')

    host = sys.argv[1]
    database = sys.argv[2]
    schema1 = sys.argv[3]
    schema2 = sys.argv[4]
    username = sys.argv[5]
    password = sys.argv[6]
    table1 = sys.argv[7]
    table2 = sys.argv[8]

    print 'program started'

    out_path = r'\\storage1\gis\temp\output.shp'
    new_dir = r'\\storage1\gis\temp'
    if not os.path.exists(new_dir):
        os.makedirs(new_dir)

    pgsql2shp = 'pgsql2shp -f %s\\%s -h %s -p 5432 -P %s -u %s %s %s.%s' % (new_dir, table1, host, password, username, database, schema1, table1)
    print pgsql2shp 
    logger.info('Return code pgsql2shp invocation 1: '+str(subprocess.Popen(pgsql2shp, shell=True).wait()))
    pgsql2shp = 'pgsql2shp -f %s\\%s -h %s -p 5432 -P %s -u %s %s %s.%s' % (new_dir, table2, host, password, username, database, schema2, table2)
    print pgsql2shp
    logger.info('Return code pgsql2shp invocation 2: '+str(subprocess.Popen(pgsql2shp, shell=True).wait()))
    print('Argument:'+ new_dir+'\\'+table1+'.shp'+' , '+ new_dir+'\\'+table2+'.shp')
    arcpy.Intersect_analysis([new_dir+'\\'+table1+'.shp', new_dir+'\\'+table2+'.shp'], out_path, "ALL")

    arcpy.AddField_management(out_path, "intersected_area", "DOUBLE")
    arcpy.AddField_management(out_path, "dist_perct", "DOUBLE")
    arcpy.CalculateField_management(out_path, "intersected_area","!SHAPE.AREA@SQUAREMETERS!", "PYTHON_9.3")

    formula = """def findArea(area_inter, area):
                    return float((float(area_inter/area))*100.0)"""


    arcpy.CalculateField_management(out_path, "dist_perct", "findArea(!intersected_area!, !AREA!)", "PYTHON_9.3", formula)

    #export shapefiles to database
    shp2pgsql = r"shp2pgsql -d -s 48591 "+out_path+" johnsmith.intersected"
    psql = "psql -U johnsmith -h example.org -d exampledb"
    logger.info("Running shp2pgsql | psql")
    sts = subprocess.Popen(shp2pgsql +" | "+psql, shell=True).wait()
    logger.info("Remove shapefiles")
    shutil.rmtree(new_dir)
    print 'program finished'
    logger.info("Done shp intr arcpy")
"""!
@endcond
"""
\$\endgroup\$
4
\$\begingroup\$
  1. I know, that you know, that your code is messy, but I'm still going to give some tips on your coding style. Here are a few tips I have on what can be improved there.

    1. Don't use % for string formatting, as it is deprecated. You should be using str.format() instead. Here's an example: print "Hello {0}".format("world")
    2. You can remove the docstring at the end of your file containing """!@endcond""". This does nothing, and serves no purpose.
    3. Separate this out into different functions. In it's current state, it's really hard to read as one large block of code.
    4. Add more comments. Comments will make your code much clearer, and easier to read. Functions and classes should use docstrings, over regular inline comments.
  2. Don't import multiple modules on one line. If an error occurs with one module, it can be hard to distinguish where the error came from.

  3. Some variable names could be better. For example, the names sts, or shp2pgsql provide no useful information. Try to find any other names with not so great names, and see how you can improve them.
  4. I'm not an expert on asynchronous programming, or any of the modules provided in Python made for this, but if you want to do a task like this, Python 3.5 will include the async and await keywords built for asynchronous programming.

The official style guide for Python, PEP8, can be found here.

\$\endgroup\$
  • \$\begingroup\$ "I know, that you know, that your code is messy, but I'm still going to give some tips on your coding style...Don't use % for string formatting, as it is deprecated." feel free to as I'm new to Python and I was instructed to use an older file that contained % so I thought that was pythonic. \$\endgroup\$ – Celeritas Jun 26 '15 at 21:20
  • \$\begingroup\$ @Celeritas Well, you shouldn't use %. str.format() was introduced in Python 2.6, and I don't think backwards compatibility beyond that is really necessary. \$\endgroup\$ – Ethan Bierlein Jun 26 '15 at 21:23
  • \$\begingroup\$ Does using % instead of str.format() somehow help to prevent SQL injection? \$\endgroup\$ – Celeritas Nov 4 '15 at 3:59
  • \$\begingroup\$ @Celeritas No. The formatting operator % has simply been deprecated since Python 2.6. \$\endgroup\$ – Ethan Bierlein Nov 4 '15 at 12:53

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.