I don't have any experience with arcpy, so possibly some of this may be invalid.
Mostly the code is very good: It's concise; it's clear enough what it does without lot's of documentation, and it has the documentation it needs.
Exactly what makes code "good" depends on how it's going to be used. This looks like a standalone utility you use at your work, so keeing it brief and easy to maintain is probably worth more that thoroughly documenting everything and enforcing type constraints. Not everything below will be applicable to you.
Context:
- Use python 3.x if at all possible.
- It's good that you've already put most of your code inside a function; now consider using the main method pattern.
- The comment about truncating the table is good. You could move it to a confirmation prompt that would be shown to the user at runtime.
Style:
- If you have parens of any kind (
({[), you don't need to use \ to break your lines.
- Your variable names could be more descriptive.
- Use snake-case:
this_is_a_variable
- Be more aggressive about breaking your lines. A reader should rarely, if ever, need to scroll sideways.
- Use parens for print statements.
print("message") This is required in python 3.
- Consider using
"string".format() instead of string concatenation. (It make more of a difference in more complicated situations.)
- You could save yourself a level of indentation by doing both your DB connections in a single with statement. (but see below)
- The warning about truncating the table could be moved to a docstring.
- The standard indentation scheme is 4 spaces per level of indentation, and brackets & parens close on the same level of indentation as the line they were opened on:
lstFC = [
["<OWNER>.<SIDEWALK>", "<OWNER>.S_SIDEWLK_MIDPOINT", "SDW_ID" ],
["<OWNER>.<SEWER>" , "<OWNER>.S_SEWER_MIDPOINT" , "SEWER_ID" ],
["<OWNER>.<ROAD>" , "<OWNER>.S_ROAD_MIDPOINT" , "ROAD_ID" ],
]
Architecture:
- It looks like you could make the string literals in
lstFC less verbose, by pulling out the common patterns. But that might not be a good idea if those patterns are likely to fail in the future.
- Instead of assigning
inputFC, targetFC, and idField in three steps, use unpacking to do them all at once.
- Alternately, you could save yourself the comment about what's in
lstFC by having a class that exposes those as instance properties.
- I think it would look better, and be more flexable, to move the
for fc in lstFC: loop outside def ReloadMidpoints():, so you'll be calling the function inside the loop.
- There's probably a way to do
i_rows.insertRows(rows). That way you could first load all the data, then do all the transformations, then write all the data. You'd be making a single large request against the DB, which would likely be faster, and you'd have already succeeded or failed at all of the processing before you ever touched the database. You can even avoid having two connections going on at once.
- Speaking of DB safety, consider using a transaction? I don't know how that's done in arcpy.
- A lot of database systems support a command called
REPLACE, which is like INSERT except it will delete conflicting data before inserting the new rows. If that's an option it might be better than the truncate and rebuild pattern you're using.
To summarize a lot of what I've written:
#!/usr/bin/env python3
import arcpy
DESCRIPTION = """
Transcribe the midpoints of source feature classes
to their respective target feature classes.
This script only works for polyline feature classes (not polygons or points).
CAUTION! The target FCs will get truncated!
All records in the target FCs will be deleted.
"""
default_connection = "Database Connections\\<YOUR CONNECTION.sde>\\"
default_feature_classes = [
("<OWNER>.<SIDEWALK>", "<OWNER>.S_SIDEWLK_MIDPOINT", "SDW_ID"),
("<OWNER>.<SEWER>", "<OWNER>.S_SEWER_MIDPOINT", "SEWER_ID"),
("<OWNER>.<ROAD>", "<OWNER>.S_ROAD_MIDPOINT", "ROAD_ID"),
]
def main():
if(_confirm()):
for (input_FC, target_FC, id_field) in default_feature_classes:
reload_midpoints(
default_connection + input_FC,
default_connection + target_FC,
id_field
)
print(_main_success_message)
def _confirm():
print(DESCRIPTION)
input(_confirmation_prompt)
return True
def midpoint(row):
return row[0].positionAlongLine(0.50,True).firstPoint
def reload_midpoints(input_connection, target_connection, id_field):
"""Caution!
The target FCs will get truncated! All records in the target FCs will be deleted."""
new_rows = []
with arcpy.da.SearchCursor(input_connection, ['SHAPE@', id_field]) as s_rows:
new_rows = [
[(m.X, m.Y), s_rows[1], m.X, m.Y]
for m in map(midpoint, s_rows)
]
if(new_rows):
arcpy.TruncateTable_management(target_connection)
print(_truncation_success_message.format(target_connection))
with arcpy.da.InsertCursor(
target_connection,
['SHAPE@', id_field, "MIDPOINT_X","MIDPOINT_Y"]
) as i_rows:
i_rows.insertRows(new_rows)
print(_midpoints_reloaded_message.format(target_connection, input_connection))
else:
print(_no_rows_message)
print("")
_truncation_success_message = "<{0}> has been truncated."
_midpoints_reloaded_message = "<{0}> has been reloaded with midpoints that were generated from <{1}>."
_no_rows_message = "There are no new rows; doing nothing."
_confirmation_prompt = "Press Enter to continue..."
_main_success_message = "Complete."
if __name__ == "__main__":
main()
```
