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 Logging to Geolocation #185

Closed
wants to merge 12 commits into from
Closed

Add Logging to Geolocation #185

wants to merge 12 commits into from

Conversation

mgupta27
Copy link
Contributor

@mgupta27 mgupta27 commented Jul 5, 2024

Add Logging to Geolocation

Key Changes

  • Added logging to the geolocation module
  • Added string representation for DetectionInWorld

Testing

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

Output from log file

06:32:52: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | create | 43] geolocation logger initialized
06:32:52: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | run | 323] <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
06:32:52: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | run | 323] <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
06:32:52: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | run | 323] <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
06:32:53: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | run | 323] <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
06:32:53: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | run | 323] <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
06:32:53: [INFO] [C:\Users\Mihir\vscode_workspace\WARG\Repositories\computer-vision-python\modules\geolocation\geolocation.py | run | 323] <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
Contributor

@TongguangZhang TongguangZhang left a comment

Choose a reason for hiding this comment

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

reviewed

Comment on lines 185 to 188
except:
# TODO: Logging
frame = inspect.currentframe()
self.__logger.error("could not get perspective transform matrix", frame)
return False, None
Copy link
Contributor

Choose a reason for hiding this comment

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

except cv2.error as e:
   # log the specific cv2 error
    return False, None
except Exception as e:
    # log the specific unexpected error type
    return False, None

Comment on lines 323 to 325
for detection_in_world in detections_in_world:
frame = inspect.currentframe()
self.__logger.info(detection_in_world, frame)
Copy link
Contributor

Choose a reason for hiding this comment

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

we should log each one as it's created in the for loop above (after we append it to the list

Xierumeng
Xierumeng previously approved these changes Jul 5, 2024
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.

Conditionally approved.

@@ -45,6 +57,10 @@ def create(
# Image space to camera space
result, value = camera_intrinsics.camera_space_from_image_space(source[0], source[1])
if not result:
frame = inspect.currentframe()
geolocation_logger.error(
f"rotated source vector could not be created for source: {source}", frame
Copy link
Collaborator

Choose a reason for hiding this comment

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

Capitalize. Same for logging statements below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I noticed in main the logger outpus main logger initialized which is why I did all lowercase. Should those be changed as well?

@Xierumeng Xierumeng dismissed their stale review July 6, 2024 00:18

Checks failing

@mgupta27 mgupta27 mentioned this pull request Sep 13, 2024
@maxlou05 maxlou05 closed this Oct 2, 2024
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.

4 participants