-
Notifications
You must be signed in to change notification settings - Fork 6
Accept numpy images #23
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
Changes from all commits
ba08a2e
4b37837
6f0f7c5
a06183b
f0322d3
f221cd1
52e0517
e17496d
ba9e69c
b49a13f
573a76a
9a1e744
098d0a8
d856739
1f148df
3765348
080858d
f1895d1
4f18886
d775494
4fe4ef4
bbb125e
5c2c547
43e2f51
f2fb3e4
7a2c1f4
2fca73b
c5956b3
1247262
eeb2500
ff75013
68440ce
32f824f
f45cae9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,14 @@ jobs: | |
| run-tests: | ||
| runs-on: ubuntu-20.04 | ||
| strategy: | ||
| fail-fast: true | ||
| # It's totally debatable which is better here: fail-fast or not. | ||
| # Failing fast will use fewer cloud resources, in theory. | ||
| # But if the tests are slightly flaky (fail to pip install something) | ||
| # Then one flaky install kills lots of jobs that need to be redone. | ||
| # So the efficiency argument has its limits | ||
| # Failing slow is clearer about what's going on. | ||
| # This is pretty unambiguous, so we're going with it for now. | ||
| fail-fast: false | ||
| matrix: | ||
| python-version: [ | ||
| #"3.6", # Default on Ubuntu18.04 but openapi-generator fails | ||
|
|
@@ -15,6 +22,8 @@ jobs: | |
| "3.10", | ||
| "3.11", | ||
| ] | ||
| install_numpy: [ true, false ] | ||
| install_pillow: [ true, false ] | ||
| env: | ||
| # This is associated with the "sdk-integ-test" user, credentials on 1password | ||
| GROUNDLIGHT_API_TOKEN: ${{ secrets.GROUNDLIGHT_API_TOKEN }} | ||
|
|
@@ -32,5 +41,15 @@ jobs: | |
| pip install -U pip | ||
| pip install poetry | ||
| poetry install | ||
| - name: setup environment | ||
| run: make install | ||
| - name: install numpy | ||
| if: matrix.install_numpy | ||
| run: | | ||
| poetry run pip install numpy | ||
| - name: install pillow | ||
| if: matrix.install_pillow | ||
| run: | | ||
| poetry run pip install pillow | ||
|
Comment on lines
+46
to
+53
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the poetry-native way to accomplish this would be "optional groups" (In v1.2+ https://python-poetry.org/docs/managing-dependencies/#optional-groups, or "extras" in v1.1: https://python-poetry.org/docs/1.1/pyproject#extras). |
||
| - name: run tests | ||
| run: make test-integ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,8 @@ | |
| from openapi_client.api.image_queries_api import ImageQueriesApi | ||
| from openapi_client.model.detector_creation_input import DetectorCreationInput | ||
|
|
||
| from groundlight.images import buffer_from_jpeg_file | ||
| from groundlight.images import buffer_from_jpeg_file, jpeg_from_numpy | ||
| from groundlight.optional_imports import np | ||
|
|
||
| API_TOKEN_WEB_URL = "https://app.groundlight.ai/reef/my-account/api-tokens" | ||
| API_TOKEN_VARIABLE_NAME = "GROUNDLIGHT_API_TOKEN" | ||
|
|
@@ -113,7 +114,7 @@ def list_image_queries(self, page: int = 1, page_size: int = 10) -> PaginatedIma | |
| def submit_image_query( | ||
| self, | ||
| detector: Union[Detector, str], | ||
| image: Union[str, bytes, BytesIO, BufferedReader], | ||
| image: Union[str, bytes, BytesIO, BufferedReader, np.ndarray], | ||
| wait: float = 0, | ||
| ) -> ImageQuery: | ||
| """Evaluates an image with Groundlight. | ||
|
|
@@ -139,9 +140,11 @@ def submit_image_query( | |
| elif isinstance(image, BytesIO) or isinstance(image, BufferedReader): | ||
| # Already in the right format | ||
| image_bytesio = image | ||
| elif isinstance(image, np.ndarray): | ||
| image_bytesio = BytesIO(jpeg_from_numpy(image)) | ||
| else: | ||
| raise TypeError( | ||
| "Unsupported type for image. We only support JPEG images specified through a filename, bytes, BytesIO, or BufferedReader object." | ||
| "Unsupported type for image. We only support numpy arrays (3,W,H) or JPEG images specified through a filename, bytes, BytesIO, or BufferedReader object." | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a world where we expose an error with "
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's pretty universal to |
||
| ) | ||
|
|
||
| raw_img_query = self.image_queries_api.submit_image_query(detector_id=detector_id, body=image_bytesio) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| """We use a trick to check if libraries like numpy are installed or not. | ||
| If they are, we make it available as normal. | ||
| If not, we set it up as a shim object which still lets type-hinting work properly, | ||
| but will fail at runtime if you try to use it. | ||
|
|
||
| This can be confusing, but hopefully the errors are explicit enough to be | ||
| clear about what's happening, and it makes the code which hopes numpy is installed | ||
| look readable. | ||
| """ | ||
|
|
||
|
|
||
| class UnavailableModule(type): | ||
| """Represents a module that is not installed or otherwise unavailable at runtime. | ||
| Attempting to access anything in this object raises the original exception | ||
| (ImportError or similar) which happened when the optional library failed to import. | ||
|
|
||
| Needs to subclass type so that it works for type-hinting. | ||
| """ | ||
|
|
||
| def __new__(cls, exc): | ||
| out = type("UnavailableModule", (), {}) | ||
| out.exc = exc | ||
| return out | ||
|
|
||
| def __getattr__(self, key): | ||
| # TODO: This isn't getting called for some reason. | ||
| raise RuntimeError("attempt to use module that failed to load") from self.exc | ||
|
|
||
|
|
||
| try: | ||
| import numpy as np | ||
|
|
||
| MISSING_NUMPY = False | ||
| except ImportError as e: | ||
| np = UnavailableModule(e) | ||
| # Expose np.ndarray so type-hinting looks normal | ||
| np.ndarray = np | ||
| MISSING_NUMPY = True | ||
|
|
||
| try: | ||
| import PIL | ||
| from PIL import Image | ||
|
|
||
| MISSING_PIL = False | ||
| except ImportError as e: | ||
| PIL = UnavailableModule(e) | ||
| Image = PIL | ||
| MISSING_PIL = True | ||
|
|
||
|
|
||
| __all__ = ["np", "PIL", "Image", "MISSING_NUMPY", "MISSING_PIL"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import pytest | ||
|
|
||
| from groundlight.images import * | ||
| from groundlight.optional_imports import * | ||
|
|
||
|
|
||
| @pytest.mark.skipif(MISSING_NUMPY or MISSING_PIL, reason="Needs numpy and pillow") | ||
| def test_jpeg_from_numpy(): | ||
| np_img = np.random.uniform(0, 255, (480, 640, 3)) | ||
| jpeg1 = jpeg_from_numpy(np_img) | ||
| assert len(jpeg1) > 500 | ||
|
|
||
| np_img = np.random.uniform(0, 255, (768, 1024, 3)) | ||
| jpeg2 = jpeg_from_numpy(np_img) | ||
| assert len(jpeg2) > len(jpeg1) | ||
|
|
||
| np_img = np.random.uniform(0, 255, (768, 1024, 3)) | ||
| jpeg3 = jpeg_from_numpy(np_img, jpeg_quality=50) | ||
| assert len(jpeg2) > len(jpeg3) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| from typing import Union | ||
|
|
||
| import pytest | ||
|
|
||
| from groundlight.optional_imports import UnavailableModule | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def failed_import() -> type: | ||
| e = ModuleNotFoundError("perfect_perception module does not exist") | ||
| return UnavailableModule(e) | ||
|
|
||
|
|
||
| def test_type_hints(failed_import): | ||
| # Check that the UnavailableModule class can be used in type hints. | ||
| def typed_method(foo: Union[failed_import, str]): | ||
| print(foo) | ||
|
|
||
| assert True, "Yay UnavailableModule can be used in a type hint" | ||
|
|
||
|
|
||
| @pytest.mark.skip("Would be nice if this works, but it doesn't") | ||
| def test_raises_exception(failed_import): | ||
| # We'd like the UnavailableModule object to raise an exception | ||
| # anytime you access it, where the exception is a RuntimeError | ||
| # but builds on the original ImportError so you can see what went wrong. | ||
| # The old version had this, but didn't work with modern type-hinting. | ||
| with pytest.raises(RuntimeError): | ||
| failed_import.foo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree! Diagnosing a failed run quickly is much more important than seeing a failed run quickly.