parkpow / deep-license-plate-recognition

Automatic License Plate Recognition (ALPR) or Automatic Number Plate Recognition (ANPR) software that works with any camera.
https://platerecognizer.com/
MIT License
523 stars 122 forks source link

Added accumuled flattened data of each plate #182

Closed dibonjohnseron closed 7 months ago

github-actions[bot] commented 7 months ago

Risk Level 2 - /home/runner/work/deep-license-plate-recognition/deep-license-plate-recognition/plate_recognition.py

The changes introduce conditional imports based on the Python version, which is a good practice for backward compatibility. However, the use of exit(1) directly within the recognition_api function is not ideal as it prevents the function from being used in a larger application that might want to handle errors differently. Instead, consider raising a custom exception that can be caught and handled by the caller.

Additionally, the flatten function has been modified to return a list of flattened dictionaries, which is a change in behavior that could affect existing callers. Ensure that all callers of this function are updated to handle the new return type.

Lastly, the CSV writing logic has been updated to use the keys of the first candidate as fieldnames. This assumes that all candidates have the same structure, which may not be the case. It would be safer to gather all possible keys from all candidates before writing the CSV header.

Suggested changes:

  1. Replace exit(1) with a custom exception:
class ApiException(Exception):
    pass

# ... inside recognition_api ...
if response.status_code < 200 or response.status_code > 300:
    print(response.text)
    if exit_on_error:
        raise ApiException('API response error')
  1. Ensure all callers of flatten can handle a list of dictionaries.

  2. Collect all unique keys for CSV fieldnames:

fieldnames = set()
for result in results:
    fieldnames.update(flatten(result.copy()).keys())
fieldnames = list(fieldnames)

🚫🔧📊


Powered by Code Review GPT

marcbelmont commented 7 months ago

@dev-bon Is the GPT powered review above helpful?

dibonjohnseron commented 7 months ago

@dev-bon Is the GPT powered review above helpful?

@marcbelmont In my opinion for our use-case the first suggestion is unnecessary since our script is just pretty straightforward and very explanatory already regarding stopping due to an error, also, for the second and third suggestion, as for the flatten function, in order to accumulate the results, I am saving it as a list instead to save it. So I refactored the logic in getting the fieldnames instead since we cannot use keys in the list.