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

Geolocation Logging #198

Merged
merged 6 commits into from
Sep 13, 2024
Merged

Geolocation Logging #198

merged 6 commits into from
Sep 13, 2024

Conversation

mgupta27
Copy link
Contributor

Add Logging to Geolocation

Key Changes

  • Added logging to the geolocation module
    • Note: The logs are designed to only print out one error log per run (
  • Added string representation for DetectionInWorld
  • Updated test cases

Testing

  • All unit tests pass
  • Ran test_geolocation_worker integration test works and confirm logs are logged as expected

Output from log file

05:08:50: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation_worker.py | geolocation_worker | 40] Logger initialized
05:08:50: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | run | 324] <class 'modules.detection_in_world.DetectionInWorld'>, vertices: [[100.0, -100.00000000000001], [100.00000000000001, 100.00000000000001], [-100.0, -99.99999999999999], [-100.0, 99.99999999999999]], centre: [0. 0.], label: 1, confidence: 1.0
05:08:50: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | run | 324] <class 'modules.detection_in_world.DetectionInWorld'>, vertices: [[100.0, -100.00000000000001], [100.00000000000001, 100.00000000000001], [-100.0, -99.99999999999999], [-100.0, 99.99999999999999]], centre: [0. 0.], label: 1, confidence: 1.0
05:08:50: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | run | 324] <class 'modules.detection_in_world.DetectionInWorld'>, vertices: [[100.0, -100.00000000000001], [100.00000000000001, 100.00000000000001], [-100.0, -99.99999999999999], [-100.0, 99.99999999999999]], centre: [0. 0.], label: 1, confidence: 1.0
05:08:50: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | run | 324] <class 'modules.detection_in_world.DetectionInWorld'>, vertices: [[100.0, -100.00000000000001], [100.00000000000001, 100.00000000000001], [-100.0, -99.99999999999999], [-100.0, 99.99999999999999]], centre: [0. 0.], label: 1, confidence: 1.0
05:08:50: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | run | 324] <class 'modules.detection_in_world.DetectionInWorld'>, vertices: [[100.0, -100.00000000000001], [100.00000000000001, 100.00000000000001], [-100.0, -99.99999999999999], [-100.0, 99.99999999999999]], centre: [0. 0.], label: 1, confidence: 1.0
05:08:50: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | run | 324] <class 'modules.detection_in_world.DetectionInWorld'>, vertices: [[100.0, -100.00000000000001], [100.00000000000001, 100.00000000000001], [-100.0, -99.99999999999999], [-100.0, 99.99999999999999]], centre: [0. 0.], label: 1, confidence: 1.0


Copy link
Collaborator

@Xierumeng Xierumeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed.

modules/geolocation/geolocation.py Outdated Show resolved Hide resolved
Comment on lines 178 to 180
# All exceptions must be caught and logged as early as possible
# pylint: disable-next=bare-except
except:
# TODO: Logging
# pylint: disable-next=catching-non-exception
except cv2.error as e:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this specific exception being caught instead of using the broader catch, especially since the code is identical?

Copy link
Contributor Author

@mgupta27 mgupta27 Sep 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this because of this comment. I thought this was needed because cv2.error was separate from the Exception class but it turns out cv2.error inherits from the Exception class so I removed it. Thanks for catching this.

class error(Exception):
    code: int
    err: str
    file: str
    func: str
    line: int
    msg: str

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's worth separating them out unless there is a specific use case.

modules/geolocation/geolocation.py Show resolved Hide resolved
modules/geolocation/geolocation_worker.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Xierumeng Xierumeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, thanks!

@mgupta27 mgupta27 merged commit 3079d00 into main Sep 13, 2024
1 check passed
@mgupta27 mgupta27 deleted the mihir.geolocation-logging-v2 branch September 13, 2024 04:15
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.

2 participants