- I don't see why
del jsonobject[...]
would be necessary before reassigning that key. Also, the test in the last two lines could also be simplified to something likejsonobject.pop('small_encoded_polyline', None)
if you don't care about that key, no?
Also:
- Everything could use more error checking. That is, check expected
types and values instead of falling through the
if
s ('imperial'
,'metric'
, what happens if it's neither, or both?). - Unless you really prefer
list
anddict
, it's better to use the literal syntax (fast, can't override it). - The type check for
dict
s can be better, i.e.isinstance(foo, dict)
; if you really want onlydict
, then it's still better to usetype(foo) is dict
instead of comparing strings. I'd also move that wholeif/else
into a function, e.g.maybe_load_json
or so. - The value of
document
after the assignments isn't used anywhere, so it's safe to remove, though I'd rather use that variable again instead of introducingdocument2
. - The second branch with
'small_encoded_polyline'
seems to have the wrong level of indentation? At least it's different from the first one in that it's only run if the document wasn't adict
. Assuming that it's safe to run anyway I'll change that in the code below. - The pattern
if foo in json and json[foo] is not None:
could be easier be achieved withif json.get(foo) is not None:
as the return value there defaults toNone
. - I'm not particularly fond of the habit to reuse input data like with
'points_correct'
done here. It would be cleaner just to have a listpoints_correct
and assign that to ajsonobject
key (and you know, possibly put the calculation into a separate function as well).
I might have messed up some of the logic now, but I hope you get the idea:
def maybe_load_json(document):
if isinstance(document, dict):
return document
return json.loads(document)
def cleaned_small_encoded_polylines(documents):
result = []
for document in documents:
#sometimes these document objects turn out to be dicts
#and sometimes they turn out to be strings
#even though JSON output is always the same
#is there a better way to deal with this?
document = maybe_load_json(document)
if document.get('small_encoded_polyline') is not None:
del document['small_encoded_polyline']
#I thought this line above would modify the original
#jsonobject since document is a dictionary so I should
#be working with a pointer to the original object
#but inspection of jsonobject reveals this not to be the case
result.append(document)
return result
...
r = requests.get(url, headers = {'User-Agent':UA})
jsonobject = r.json()
if jsonobject.get('laps') is not None:
if 'imperial' in jsonobject['laps']:
laps_array = jsonobject['laps']['imperial']
type = 'imperial'
if 'metric' in jsonobject['laps']:
laps_array = jsonobject['laps']['metric']
type = 'metric'
if laps_array is not None:
jsonobject['laps'][type] = cleaned_small_encoded_polylines(laps_array)
# this can't be an elif because sometimes json objects
# have both "points" and "laps"
if jsonobject.get('points') is not None:
if 'points' in jsonobject['points']:
points_correct = []
laps_array = jsonobject['points']['points']
if laps_array is not None:
jsonobject['points']['points'] = cleaned_small_encoded_polylines(laps_array)
jsonobject.pop('small_encoded_polyline', None)