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

Should prefer raise over print() + exit() #256

Closed
quantumdot opened this issue Feb 23, 2023 · 4 comments
Closed

Should prefer raise over print() + exit() #256

quantumdot opened this issue Feb 23, 2023 · 4 comments
Labels
feature request New feature or request good first issue Good for newcomers

Comments

@quantumdot
Copy link
Contributor

Describe the bug
In a few locations within this package, if some condition is not met, an error message is printed and then the interpreter is instructed to exit(). Instead it would be better to raise an exception and allow the consuming program to handle the error condition, perhaps via graceful exit itself. This especially poses an issue in scenarios involving multiprocessing as these error messages may become lost or cause a subprocess to exit but the main program continues to run. Another scenario where this causes issues is when using tqdm progress bars, which will tend to overwrite any standard output and causing the message to be lost. Finally, we lose any context for the error, since no stack trace is generated.

Here are some locations where errors are handled by printing then exiting:

  • norfair/norfair/tracker.py

    Lines 296 to 300 in 705f87c

    if np.isnan(distance_matrix).any():
    print(
    "\nReceived nan values from distance function, please check your distance function for errors!"
    )
    exit()
  • norfair/norfair/tracker.py

    Lines 487 to 491 in 705f87c

    if not isinstance(initial_detection, Detection):
    print(
    f"\n[red]ERROR[/red]: The detection list fed into `tracker.update()` should be composed of {Detection} objects not {type(initial_detection)}.\n"
    )
    exit()
  • norfair/norfair/video.py

    Lines 178 to 180 in 50f5607

    def _fail(self, msg: str):
    print(msg)
    exit()
  • norfair/norfair/utils.py

    Lines 21 to 30 in cda59f7

    def print_detection_error_message_and_exit(points):
    print("\n[red]INPUT ERROR:[/red]")
    print(
    f"Each `Detection` object should have a property `points` of shape (num_of_points_to_track, 2), not {points.shape}. Check your `Detection` list creation code."
    )
    print("You can read the documentation for the `Detection` class here:")
    print(
    "https://tryolabs.github.io/norfair/reference/tracker/#norfair.tracker.Detection\n"
    )
    exit()

To Reproduce
Any condition which would trigger one of the above conditionals.

Expected behavior
An exception would be raised, the consuming program can decide how to proceed.

Screenshots or videos
N/A

Environment (please complete the following information):

  • OS: Windows 10
  • Python version: 3.8.12
  • Norfair version: 2.2.0

Additional context

@quantumdot quantumdot added the bug Something isn't working label Feb 23, 2023
@facundo-lezama
Copy link
Collaborator

Hi @quantumdot, thanks for your observation. This behavior was defined a long time ago, and it isn't a bug, actually. Until now, we didn't have the need to change this, so we left it like this.

If you need this behavior to change, we encourage you to open a PR with the changes!

@facundo-lezama facundo-lezama added feature request New feature or request good first issue Good for newcomers and removed bug Something isn't working labels Feb 28, 2023
@quantumdot
Copy link
Contributor Author

Hi @facundo-lezama, I submitted a PR addressing this. Will you consider it?

@facundo-lezama
Copy link
Collaborator

Sorry I didn't have the time to look at it. I'll make some time this week and give you feedback.

@facundo-lezama
Copy link
Collaborator

This got merged in #259. I'm closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants