-
Notifications
You must be signed in to change notification settings - Fork 0
Sourcery refactored master branch #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to GitHub API limits, only the first 60 comments can be shown.
if left == None: | ||
if left is None: | ||
left = 0 | ||
if right == None: | ||
if right is None: | ||
right = 0 | ||
if top == None: | ||
if top is None: | ||
top = 0 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function expand
refactored with the following changes:
- Use x is None rather than x == None [×3] (
none-compare
)
if left == None: | ||
if left is None: | ||
left = 0 | ||
if right == None: | ||
if right is None: | ||
right = 0 | ||
if top == None: | ||
if top is None: | ||
top = 0 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function add_border
refactored with the following changes:
- Use x is None rather than x == None [×3] (
none-compare
)
if settings.use_cache: | ||
if response_json is None: | ||
log('Saved nothing to cache because response_json is None') | ||
else: | ||
# create the folder on the disk if it doesn't already exist | ||
if not os.path.exists(settings.cache_folder): | ||
os.makedirs(settings.cache_folder) | ||
if not settings.use_cache: | ||
return | ||
if response_json is None: | ||
log('Saved nothing to cache because response_json is None') | ||
else: | ||
# create the folder on the disk if it doesn't already exist | ||
if not os.path.exists(settings.cache_folder): | ||
os.makedirs(settings.cache_folder) | ||
|
||
# hash the url (to make filename shorter than the often extremely | ||
# long url) | ||
filename = hashlib.md5(url.encode('utf-8')).hexdigest() | ||
cache_path_filename = os.path.join(settings.cache_folder, os.extsep.join([filename, 'json'])) | ||
# hash the url (to make filename shorter than the often extremely | ||
# long url) | ||
filename = hashlib.md5(url.encode('utf-8')).hexdigest() | ||
cache_path_filename = os.path.join(settings.cache_folder, os.extsep.join([filename, 'json'])) | ||
|
||
# dump to json, and save to file | ||
json_str = make_str(json.dumps(response_json)) | ||
with io.open(cache_path_filename, 'w', encoding='utf-8') as cache_file: | ||
cache_file.write(json_str) | ||
# dump to json, and save to file | ||
json_str = make_str(json.dumps(response_json)) | ||
with io.open(cache_path_filename, 'w', encoding='utf-8') as cache_file: | ||
cache_file.write(json_str) | ||
|
||
log('Saved response to cache file "{}"'.format(cache_path_filename)) | ||
log(f'Saved response to cache file "{cache_path_filename}"') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function save_to_cache
refactored with the following changes:
- Add guard clause (
last-if-guard
) - Replace call to format with f-string (
use-fstring-for-formatting
)
log('Retrieved response from cache file "{}" for URL "{}"'.format(cache_path_filename, url)) | ||
log( | ||
f'Retrieved response from cache file "{cache_path_filename}" for URL "{url}"' | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function get_from_cache
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
# if first token is 'Currently', it is currently running a query so | ||
# check back in recursive_delay seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function get_pause_duration
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
This removes the following comments ( why? ):
# check back in recursive_delay seconds
# if first token is 'Currently', it is currently running a query so
multipoly = geometry.difference(quadrats) | ||
|
||
return multipoly | ||
return geometry.difference(quadrats) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function quadrat_cut_geometry
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
if len(elements) < 1: | ||
if not elements: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function create_graph
refactored with the following changes:
- Simplify sequence length comparison (
simplify-len-comparison
)
log('Created bounding box {} meters in each direction from {} and projected it: {},{},{},{}'.format(distance, point, north, south, east, west)) | ||
log( | ||
f'Created bounding box {distance} meters in each direction from {point} and projected it: {north},{south},{east},{west}' | ||
) | ||
|
||
else: | ||
# if project_utm is False, project back to lat-long then get the | ||
# bounding coordinates | ||
buffer_latlong, _ = project_geometry(buffer_proj, crs=crs_proj, to_latlong=True) | ||
west, south, east, north = buffer_latlong.bounds | ||
log('Created bounding box {} meters in each direction from {}: {},{},{},{}'.format(distance, point, north, south, east, west)) | ||
log( | ||
f'Created bounding box {distance} meters in each direction from {point}: {north},{south},{east},{west}' | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function bbox_from_point
refactored with the following changes:
- Replace call to format with f-string [×2] (
use-fstring-for-formatting
)
if return_coords: | ||
return G, point | ||
else: | ||
return G | ||
return (G, point) if return_coords else G |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function graph_from_address
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
if isinstance(query, str) or isinstance(query, dict): | ||
if isinstance(query, (str, dict)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function graph_from_place
refactored with the following changes:
- Merge isinstance calls (
merge-isinstance
)
pause_duration=0.02): # pragma: no cover | ||
pause_duration=0.02): # pragma: no cover |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function add_node_elevations
refactored with the following changes:
- Replace call to format with f-string [×5] (
use-fstring-for-formatting
)
by_bbox = not (north is None or south is None or east is None or west is None) | ||
by_bbox = ( | ||
north is not None | ||
and south is not None | ||
and east is not None | ||
and west is not None | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function osm_footprints_download
refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan
) - Replace if statement with if expression (
assign-if-exp
) - Hoist statements out of for/while loops [×2] (
hoist-statement-from-loop
) - Remove redundant conditional (
remove-redundant-if
) - Replace call to format with f-string (
use-fstring-for-formatting
)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function create_footprints_gdf
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
@@ -257,7 +259,6 @@ def responses_to_dicts(responses, footprint_type): | |||
if 'type' in element and element['type']=='node': | |||
vertices[element['id']] = {'lat' : element['lat'], | |||
'lon' : element['lon']} | |||
# WAYS - both open and closed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function responses_to_dicts
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
This removes the following comments ( why? ):
# RELATIONS
# WAYS - both open and closed
# Log any other Elements found in the response
log('Polygon has invalid geometry: {}'.format(footprint_key)) | ||
# OPEN WAYS | ||
log(f'Polygon has invalid geometry: {footprint_key}') | ||
else: | ||
try: | ||
footprint_geometry = LineString([(vertices[node]['lon'], vertices[node]['lat']) for node in footprint_val['nodes']]) | ||
except Exception: | ||
log('LineString has invalid geometry: {}'.format(footprint_key)) | ||
log(f'LineString has invalid geometry: {footprint_key}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function create_footprint_geometry
refactored with the following changes:
- Replace call to format with f-string [×2] (
use-fstring-for-formatting
)
This removes the following comments ( why? ):
# OPEN WAYS
query_str = query_template.format(amenities="|".join(amenities), north=north, south=south, east=east, west=west, | ||
timeout=timeout, maxsize=maxsize) | ||
return query_template.format( | ||
amenities="|".join(amenities), | ||
north=north, | ||
south=south, | ||
east=east, | ||
west=west, | ||
timeout=timeout, | ||
maxsize=maxsize, | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function parse_poi_query
refactored with the following changes:
- Lift return into if (
lift-return-into-if
)
elif not (north is None or south is None or east is None or west is None): | ||
elif ( | ||
north is not None | ||
and south is not None | ||
and east is not None | ||
and west is not None | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function osm_poi_download
refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
This removes the following comments ( why? ):
# Get the POIs
coords = {} | ||
for result in osm_response['elements']: | ||
if 'type' in result and result['type'] == 'node': | ||
coords[result['id']] = {'lat': result['lat'], | ||
'lon': result['lon']} | ||
return coords | ||
return { | ||
result['id']: {'lat': result['lat'], 'lon': result['lon']} | ||
for result in osm_response['elements'] | ||
if 'type' in result and result['type'] == 'node' | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function parse_nodes_coords
refactored with the following changes:
- Convert for loop into dictionary comprehension (
dict-comprehension
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
log('Polygon has invalid geometry: {}'.format(nodes)) | ||
log(f'Polygon has invalid geometry: {nodes}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function parse_polygonal_poi
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
log('Point has invalid geometry: {}'.format(response['id'])) | ||
log(f"Point has invalid geometry: {response['id']}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function parse_osm_node
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
multipoly = MultiPolygon(list(gdf_clean['geometry'])) | ||
return multipoly | ||
return MultiPolygon(list(gdf_clean['geometry'])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function invalid_multipoly_handler
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
log("Could not handle OSM 'relation': {}".format(relation['id'])) | ||
log(f"Could not handle OSM 'relation': {relation['id']}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function parse_osm_relations
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
# Parse POI area Polygon | ||
poi_area = parse_polygonal_poi(coords=coords, response=result) | ||
if poi_area: | ||
if poi_area := parse_polygonal_poi(coords=coords, response=result): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function create_poi_gdf
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
This removes the following comments ( why? ):
# Combine GeoDataFrames
# Parse POI area Polygon
# if to_crs was not passed-in, calculate the centroid of the geometry to | ||
# determine UTM zone | ||
elif to_latlong: | ||
# if to_latlong is True, project the gdf to latlong | ||
latlong_crs = settings.default_crs | ||
projected_gdf = gdf.to_crs(latlong_crs) | ||
log('Projected the GeoDataFrame "{}" to default_crs in {:,.2f} seconds'.format(gdf.gdf_name, time.time()-start_time)) | ||
else: | ||
if to_latlong: | ||
# if to_latlong is True, project the gdf to latlong | ||
latlong_crs = settings.default_crs | ||
projected_gdf = gdf.to_crs(latlong_crs) | ||
log('Projected the GeoDataFrame "{}" to default_crs in {:,.2f} seconds'.format(gdf.gdf_name, time.time()-start_time)) | ||
else: | ||
# else, project the gdf to UTM | ||
# if GeoDataFrame is already in UTM, just return it | ||
if (gdf.crs is not None) and ('+proj=utm ' in gdf.crs): | ||
return gdf | ||
|
||
# calculate the centroid of the union of all the geometries in the | ||
# GeoDataFrame | ||
avg_longitude = gdf['geometry'].unary_union.centroid.x | ||
|
||
# calculate the UTM zone from this avg longitude and define the UTM | ||
# CRS to project | ||
utm_zone = int(math.floor((avg_longitude + 180) / 6.) + 1) | ||
utm_crs = '+proj=utm +zone={} +ellps=WGS84 +datum=WGS84 +units=m +no_defs'.format(utm_zone) | ||
|
||
# project the GeoDataFrame to the UTM CRS | ||
projected_gdf = gdf.to_crs(utm_crs) | ||
log('Projected the GeoDataFrame "{}" to UTM-{} in {:,.2f} seconds'.format(gdf.gdf_name, utm_zone, time.time()-start_time)) | ||
# else, project the gdf to UTM | ||
# if GeoDataFrame is already in UTM, just return it | ||
if (gdf.crs is not None) and ('+proj=utm ' in gdf.crs): | ||
return gdf | ||
|
||
# calculate the centroid of the union of all the geometries in the | ||
# GeoDataFrame | ||
avg_longitude = gdf['geometry'].unary_union.centroid.x | ||
|
||
# calculate the UTM zone from this avg longitude and define the UTM | ||
# CRS to project | ||
utm_zone = int(math.floor((avg_longitude + 180) / 6.) + 1) | ||
utm_crs = f'+proj=utm +zone={utm_zone} +ellps=WGS84 +datum=WGS84 +units=m +no_defs' | ||
|
||
|
||
# project the GeoDataFrame to the UTM CRS | ||
projected_gdf = gdf.to_crs(utm_crs) | ||
log('Projected the GeoDataFrame "{}" to UTM-{} in {:,.2f} seconds'.format(gdf.gdf_name, utm_zone, time.time()-start_time)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function project_gdf
refactored with the following changes:
- Merge else clause's nested if statement into elif (
merge-else-if-into-elif
) - Replace call to format with f-string (
use-fstring-for-formatting
)
This removes the following comments ( why? ):
# if to_crs was not passed-in, calculate the centroid of the geometry to
# determine UTM zone
gdf_nodes.gdf_name = '{}_nodes'.format(G_proj.name) | ||
gdf_nodes.gdf_name = f'{G_proj.name}_nodes' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function project_graph
refactored with the following changes:
- Replace call to format with f-string [×3] (
use-fstring-for-formatting
) - Simplify sequence length comparison (
simplify-len-comparison
)
intersection_centroids = unified_intersections.centroid | ||
return intersection_centroids | ||
return unified_intersections.centroid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function clean_intersections
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
@@ -91,7 +91,6 @@ def basic_stats(G, area=None, clean_intersects=False, tolerance=15, | |||
area in square kilometers | |||
""" | |||
|
|||
sq_m_in_sq_km = 1e6 #there are 1 million sq meters in 1 sq km |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function basic_stats
refactored with the following changes:
- Move assignments closer to their usage (
move-assign
) - Replace unneeded comprehension with generator [×2] (
comprehension-to-generator
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
This removes the following comments ( why? ):
# assemble the results
@@ -294,7 +296,6 @@ def extended_stats(G, connectivity=False, anc=False, ecc=False, bc=False, cc=Fal | |||
|
|||
""" | |||
|
|||
stats = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function extended_stats
refactored with the following changes:
- Move assignment closer to its usage within a block (
move-assign-in-block
) - Merge dictionary assignment with declaration [×2] (
merge-dict-assign
)
todays_date = dt.datetime.today().strftime('%Y_%m_%d') | ||
log_filename = os.path.join(settings.logs_folder, '{}_{}.log'.format(filename, todays_date)) | ||
todays_date = dt.datetime.now().strftime('%Y_%m_%d') | ||
log_filename = os.path.join( | ||
settings.logs_folder, f'{filename}_{todays_date}.log' | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function get_logger
refactored with the following changes:
- Replace datetime.datetime.today() with datetime.datetime.now() (
use-datetime-now-not-today
) - Replace call to format with f-string (
use-fstring-for-formatting
)
else: | ||
# if the graph is not connected retain only the largest weakly connected component | ||
if not nx.is_weakly_connected(G): | ||
elif not nx.is_weakly_connected(G): | ||
|
||
# get all the weakly connected components in graph then identify the largest | ||
wccs = nx.weakly_connected_components(G) | ||
largest_wcc = max(wccs, key=len) | ||
G = induce_subgraph(G, largest_wcc) | ||
# get all the weakly connected components in graph then identify the largest | ||
wccs = nx.weakly_connected_components(G) | ||
largest_wcc = max(wccs, key=len) | ||
G = induce_subgraph(G, largest_wcc) | ||
|
||
msg = ('Graph was not connected, retained only the largest weakly ' | ||
'connected component ({:,} of {:,} total nodes) in {:.2f} seconds') | ||
log(msg.format(len(list(G.nodes())), original_len, time.time()-start_time)) | ||
msg = ('Graph was not connected, retained only the largest weakly ' | ||
'connected component ({:,} of {:,} total nodes) in {:.2f} seconds') | ||
log(msg.format(len(list(G.nodes())), original_len, time.time()-start_time)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function get_largest_component
refactored with the following changes:
- Merge else clause's nested if statement into elif (
merge-else-if-into-elif
)
This removes the following comments ( why? ):
# if the graph is not connected retain only the largest weakly connected component
Sourcery Code Quality Report✅ Merging this PR will increase code quality in the affected files by 0.28%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
Branch
master
refactored by Sourcery.If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
master
branch, then run:Help us improve this pull request!