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

Added numpy and imutils imports to remote_detect() #435

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cmilanf
Copy link
Contributor

@cmilanf cmilanf commented Nov 1, 2024

remote_detect() was missing numpy and imutils imports, throwing the following non-critical erros in the log:

ERR zm_detect.py:175 [Error during image grab: name 'np' is not defined]
ERR zm_detect.py:176 [Error during image grab: name 'imutils' is not defined]

This PR fixes the issues by adding the missing imports in zm_detect.py.

@connortechnology
Copy link
Member

I believe I moved these because it was very slow to startup for some task that didn't require them. So I moved them down to where they are actually required. Clearly I didn't do a very good job, but I'm loath to just stick them back at the top.

What about if we move remote_detect below main_handler so that by the time it gets called, numpy has been loaded.

@cmilanf
Copy link
Contributor Author

cmilanf commented Nov 3, 2024

It may be worth trying:

  1. Adding the imports at the begining and measure again performance impact if the last time doing so was with an old Python version. I am currently running ZMES with Python 3.10.
  2. Moving remote_detect() is really simple and straight forward, I could try that on my setup.
  3. Implement lazy loading for the imports, but that is a Python pending subject.

I can try 2 very easily. For 1, as I am a (happy) remote ML API user, my mesures wouldn't be that useful; and about 3 I wouldn't be keen to add more library dependencies to the code.

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