I wrote this first function that use a JSON datafile as an input
def jsonfile_loader_work_rate(filename):
'''Load the json file, to retrieve the interventions id and the cars id
'''
try:
with open(filename, "r") as jsonfile:
json_data = json.load(jsonfile)
cars = [
json_data["cars"][val]["id"]
for val in range(len(json_data["cars"]))]
interventions = [
json_data["interventions"][val]["id"]
for val in range(len(json_data["interventions"]))
]
return cars,interventions
except:
print('No date or no file')
This is the json file
{
"interventions": [
{
"id": 1, "name": "batteria cambio",
"parts_spec": [
{"type": "battery", "count": 1}
]
},
{
"id": 2, "name": "Cambio de lo disc y de la plakette",
"parts_spec": [
{"type": "front_brake_pad", "count": 2},
{"type": "front_brake_disc", "count": 2}
]
}
],
"cars": [
{ "id": 1, "manufacturer": "Peugeot", "model": "307 CC", "version": "2.0 16v 140cv"},
{ "id": 2, "manufacturer": "BMW", "model": "Série 3", "version": "318i 136cv"},
{ "id": 3, "manufacturer": "Toyota", "model": "Rav 4 3", "version": "2.2 D-4D 4WD 136cv"}
],
"parts": [
{"id": 1, "type": "battery", "car_id": 1, "price": "95.99"},
{"id": 10, "type": "battery", "car_id": 1, "price": "106.00"},
{"id": 11, "type": "battery", "car_id": 1, "price": "84.40"},
{"id": 12, "type": "battery", "car_id": 2, "price": "121.00"},
{"id": 13, "type": "battery", "car_id": 2, "price": "150.05"},
{"id": 4, "type": "battery", "car_id": 2, "price": "123.10"},
{"id": 14, "type": "battery", "car_id": 3, "price": "109.00"},
{"id": 7, "type": "battery", "car_id": 3, "price": "97.99"},
{"id": 14, "type": "battery", "car_id": 3, "price": "200.00"},
{"id": 2, "type": "front_brake_pad", "car_id": 1, "price": "17.20"},
{"id": 3, "type": "front_brake_disc", "car_id": 1, "price": "80.00"},
{"id": 5, "type": "front_brake_pad", "car_id": 2, "price": "24.5"},
{"id": 6, "type": "front_brake_disc", "car_id": 2, "price": "110.4"},
{"id": 8, "type": "front_brake_pad", "car_id": 3, "price": "30.34"},
{"id": 9, "type": "front_brake_disc", "car_id": 3, "price": "120.50"}
],
"workshops": [
{"id": 1, "name": "XYZ", "hourly_rate": "67.00", "preferred_part_price": "median"},
{"id": 2, "name": "ZYX", "hourly_rate": "72.99", "preferred_part_price": "most_expensive"},
{"id": 3, "name": "YZX", "hourly_rate": "63.50", "preferred_part_price": "cheapest"}
]
}
When I'm running jsonfile_loader_work_rate('data.json')
, it gives me the following
result ([1, 2, 3], [1, 2])
.
I'm then using the output of the function, in another function
def retrieve_with_api_url_labour_time(loader_cars_interv,url):
cars, interventions = loader_cars_interv
spent_time_by_op_results = []
for val_car in cars:
for val_interv in interventions:
results = requests.get(f"{url}/{val_car}/{val_interv}")
time_load = json.loads(results.text)
time_load = time_load["labourTime"]
spent_time_by_op_results.append(
{"car_id": val_car,
"interv_id": val_interv,
"time_spent": time_load}
)
return spent_time_by_op_results
I cannot give you the URL as I don't want to display it publicly.
However, I can give you a sample of the results when I query the API
{"labourTime":"00:20:00"}
Below is the final desired result. My two functions, imbricated, provides it.
[{'car_id': 1, 'interv_id': 1, 'time_spent': '00:20:00'}, {'car_id': 1, 'interv_id': 2, 'time_spent': '01:10:00'}, {'car_id': 2, 'interv_id': 1, 'time_spent': '00:25:00'}, {'car_id': 2, 'interv_id': 2, 'time_spent': '01:40:00'}, {'car_id': 3, 'interv_id': 1, 'time_spent': '00:29:00'}, {'car_id': 3, 'interv_id': 2, 'time_spent': '01:55:00'}]
I run the two functions like that
print(retrieve_with_api_url_labour_time(jsonfile_loader_work_rate('data.json'),'https://www.acme.ie/apipint'))
The question is the following: Is there another way to run the script retrieve_with_api_url_labour_time
, instead of what I'm doing?
1 Answer 1
Use PEP484 type hints.
Don't use range(len())
on your data; iterate over the inner dictionaries directly.
Never bare except
; let exceptions fall through so that they can be properly handled at the outer level. If you do want to translate internal exception traces to user-friendly messages, you can; but the way shown in the original post is not the way to do it. Among other reasons,
- The caller will not know that anything failed until it tries to use the return value and finds out - surprise - that it's
None
. - A user-break (Ctrl+C) issued during the execution of that function will be swallowed by your
except:
, when it shouldn't be. - Which is it: no data, no file, or something else? The message is both not specific enough and not accurate enough. If you handle the exception at the outer level, you should have separate
except
clauses covering the specific failure being described.
Whenever you call requests
, make sure to check for failure - which it doesn't by default.
Don't call json.loads
on a Requests response; use its own .json()
method instead.
Don't use dictionaries for internal data representation; use class instances - dataclass
or NamedTuple
being two reasonable options.
Don't use a string type for your time_spent
when it's actually a timedelta
. Incredibly, Python timedelta parsing is pretty garbage and so some simple manual parsing something like I've shown will be necessary.
Consider using generators instead of building up lists for your API function.
It's fine to return two iterables from your loader
, but they should be unpacked to two separate variables instead of passed around together.
Suggested
import json
from datetime import timedelta
from typing import Iterable, Collection, Iterator, NamedTuple
import requests as requests
def jsonfile_loader_work_rate(filename: str) -> tuple[
list[int], # car IDs
list[int], # intervention IDs
]:
"""Load the json file, to retrieve the interventions id and the cars id
"""
with open(filename, 'r') as jsonfile:
json_data = json.load(jsonfile)
cars = [
car['id']
for car in json_data['cars']
]
interventions = [
intervention['id']
for intervention in json_data['interventions']
]
return cars, interventions
class LabourTime(NamedTuple):
car_id: int
intervention_id: int
time_spent: timedelta
IMAGINARY = True
def parse_timedelta(text: str) -> timedelta:
h, m, s = (int(t) for t in text.split(':'))
return timedelta(hours=h, minutes=m, seconds=s)
def retrieve_with_api_url_labour_time(
car_ids: Iterable[int],
intervention_ids: Collection[int],
url: str,
) -> Iterator[LabourTime]:
for car_id in car_ids:
for interv_id in intervention_ids:
if IMAGINARY:
time_load = {'labourTime': '25:20:01'}
else:
with requests.get(f'{url}/{car_id}/{interv_id}') as resp:
resp.raise_for_status()
time_load = resp.json()
yield LabourTime(
car_id=car_id,
intervention_id=interv_id,
time_spent=parse_timedelta(time_load['labourTime']),
)
def main() -> None:
car_ids, intervention_ids = jsonfile_loader_work_rate('273228.json')
for labour_time in retrieve_with_api_url_labour_time(
car_ids=car_ids,
intervention_ids=intervention_ids,
url='https://www.acme.ie/apipint',
):
print(labour_time)
if __name__ == '__main__':
main()
-
\$\begingroup\$ I'd suggest (changing the signature and) returning a value from
main
for exit codes, and then modifying the call inif __name__ == '__main__':
tosys.exit(main())
; then you can return non-zero values if exceptions are raised. \$\endgroup\$ades– ades2022年01月22日 10:10:00 +00:00Commented Jan 22, 2022 at 10:10 -
\$\begingroup\$ @ades that's only really important for programs that have a non-trivial command-line interface with arguments, etc. That isn't the case here. Also note that that already happens for default exception handling: try for yourself,
python -c 'j = 0; j /= j'; echo $?
\$\endgroup\$Reinderien– Reinderien2022年01月22日 14:55:34 +00:00Commented Jan 22, 2022 at 14:55 -
\$\begingroup\$ I suggest it out of two reasons: first you can see in the OP that he wants to catch these errors (
FileNotFoundError
and something relating to time), and second I find amain() -> None
entry kind of non-standard. The second point is more subjective, but I find it strange to change OPs code to let the errors/exceptions slip by to the user without telling OP what your rationale is for that change. \$\endgroup\$ades– ades2022年01月22日 15:03:07 +00:00Commented Jan 22, 2022 at 15:03 -
\$\begingroup\$ @ades That there wasn't enough rationale is fair (edited); for everything else - it's off-topic and can land in chat.stackexchange.com/rooms/133429/… \$\endgroup\$Reinderien– Reinderien2022年01月22日 15:38:19 +00:00Commented Jan 22, 2022 at 15:38