Skip to content
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

Add split - grid_size #206

Merged
merged 2 commits into from
May 16, 2024
Merged

Add split - grid_size #206

merged 2 commits into from
May 16, 2024

Conversation

ttayson
Copy link
Collaborator

@ttayson ttayson commented May 14, 2024

Adding a grid to divide images

The feature maintains the previous pattern, dividing the image in two, and enables further division into more possibilities.

  • Add grid_size function
  • Update Manual

adding a grid to divide images
@ttayson ttayson requested a review from marcbelmont May 14, 2024 01:36
Copy link

github-actions bot commented May 14, 2024

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

The changes introduce a few potential issues that should be addressed:

  1. Exception for Missing Arguments: The exception raised when neither sdk_url nor api_key is provided should be more specific. Instead of a generic Exception, consider using ValueError which is more appropriate for invalid argument values.

    if not args.sdk_url and not args.api_key:
        raise ValueError(\"Either sdk_url or api_key must be provided\")
  2. API Key in Headers: Ensure that the API key is not logged or exposed in any way. If the API key is printed or logged somewhere in the code, it should be removed or obfuscated.

  3. Error Handling in API Calls: The code exits the script with exit(1) if the response status code is not successful and exit_on_error is True. This could be problematic if the script is part of a larger application. Consider raising an exception instead of exiting the script.

    if response.status_code < 200 or response.status_code > 300:
        print(response.text)
        if exit_on_error:
            raise RuntimeError('API call failed with status code: {}'.format(response.status_code))
  4. Text Width Calculation: The calculation of text_width seems to be incorrect as it tries to access a non-existent font.font attribute. It should be font.getsize(text) instead.

    (text_width, text_height) = font.getsize(text)
  5. Split Image Validation: The validation for args.split_x and args.split_y should check for positive values rather than just non-zero values to ensure that the image is split into a valid number of parts.

    if args.split_x <= 0 or args.split_y <= 0:
        raise ValueError(\"--split-x and --split-y must be positive integers\")

🔒🐛🚫


Powered by Code Review GPT

plate_recognition.py Outdated Show resolved Hide resolved
plate_recognition.py Outdated Show resolved Hide resolved
@danleyb2 danleyb2 requested a review from marcbelmont May 16, 2024 11:42
@marcbelmont
Copy link
Collaborator

@ttayson
Can you please check Brian's version and confirm that it works with the client's image? If everything is good, please merge and share it with the client.

@ttayson
Copy link
Collaborator Author

ttayson commented May 16, 2024

@danleyb2 @marcbelmont , I ran tests and everything worked as expected.

The minimum accepted configuration is split-x=1 and split-y=1.

For a 3x3 grid, we have (3 + 1) * (3 + 1) = 16 images.
For a 2x2 grid, we have (2 + 1) * (2 + 1) = 9 images.
For a 2x1 grid, we have (2 + 1) * (1 + 1) = 6 images.
For a 1x1 grid, we have (1 + 1) * (1 + 1) = 4 images.

Just as a suggestion, a small change from "or" to "and" might be considered, as both parameters are mandatory.

@ttayson ttayson merged commit 03d4758 into master May 16, 2024
1 check passed
@marcbelmont marcbelmont deleted the script_change_split_image branch May 17, 2024 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants