diff --git a/.github/workflows/tag.yml b/.github/workflows/tag.yml index 26f82fee6245e..1ca04f91b2192 100644 --- a/.github/workflows/tag.yml +++ b/.github/workflows/tag.yml @@ -107,6 +107,7 @@ jobs: tags: "latest, ${{env.JINA_VERSION}}, ${{env.JINA_MINOR_VERSION}}" create-release: + needs: update-docker runs-on: ubuntu-latest steps: - name: Checkout code diff --git a/jina/executors/metas.py b/jina/executors/metas.py index 480c284f02548..e8b0b11adfc2d 100644 --- a/jina/executors/metas.py +++ b/jina/executors/metas.py @@ -68,6 +68,44 @@ using :func:`BaseExecutor.load_config` from a YAML file. If a relative path is given then the root path is set to the path of the current YAML file. + Example of ``py_module`` usage: + + 1. This is a valid structure and it is RECOMMENDED: + - "my_cust_module" is a python module + - all core logic of your customized executor goes to ``__init__.py`` + - to import ``foo.py``, you can use relative import, e.g. ``from .foo import bar`` + - ``helper.py`` needs to be put BEFORE `__init__.py` in YAML ``py_modules`` + + This is also the structure given by ``jina hub new`` CLI. + + .. highlight:: text + .. code-block:: text + + my_cust_module + |- __init__.py + |- helper.py + |- config.yml + |- py_modules + |- helper.py + |- __init__.py + + 2. This is a valid structure but not recommended: + - "my_cust_module" is not a python module (lack of __init__.py under the root) + - to import ``foo.py``, you must to use ``from jinahub.foo import bar`` + - ``jinahub`` is a common namespace for all plugin-modules, not changeable. + - ``helper.py`` needs to be put BEFORE `my_cust.py` in YAML ``py_modules`` + + .. highlight:: text + .. code-block:: text + + my_cust_module + |- my_cust.py + |- helper.py + |- config.yml + |- py_modules + |- helper.py + |- my_cust.py + :type: str/List[str] :default: ``None`` diff --git a/jina/importer.py b/jina/importer.py index cc76d9a1306f6..e6c317a6717c1 100644 --- a/jina/importer.py +++ b/jina/importer.py @@ -103,7 +103,7 @@ def import_classes(namespace: str, targets=None, if k in targets: targets.remove(k) if not targets: - return getattr(mod, k) # target execs are all found and loaded, return + return getattr(mod, k) # target execs are all found and loaded, return try: # load the default request for this executor if possible from .executors.requests import get_default_reqs @@ -158,7 +158,7 @@ class ImportExtensions: """ def __init__(self, required: bool, logger=None, - help_text: str = None, pkg_name: str = None, verbose: bool=True): + help_text: str = None, pkg_name: str = None, verbose: bool = True): """ :param required: set to True if you want to raise the ModuleNotFound error @@ -276,10 +276,16 @@ def _path_import(absolute_path: str) -> Optional[ModuleType]: # I dont want to trust user path based on directory structure, "jinahub", period spec = importlib.util.spec_from_file_location('jinahub', absolute_path) module = importlib.util.module_from_spec(spec) - sys.modules[spec.name] = module # add this line + user_module_name = os.path.splitext(os.path.basename(absolute_path))[0] + if user_module_name == '__init__': + # __init__ can not be used as a module name + spec_name = spec.name + else: + spec_name = f'{spec.name}.{user_module_name}' + sys.modules[spec_name] = module spec.loader.exec_module(module) - except ModuleNotFoundError: - module = None + except Exception as ex: + raise ImportError(f'can not import module from {absolute_path}') from ex return module diff --git a/tests/integration/issues/github_1546/__init__.py b/tests/integration/issues/github_1546/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tests/integration/issues/github_1546/bad1/crafter.yml b/tests/integration/issues/github_1546/bad1/crafter.yml new file mode 100644 index 0000000000000..5234fedca6485 --- /dev/null +++ b/tests/integration/issues/github_1546/bad1/crafter.yml @@ -0,0 +1,3 @@ +!CustomCrafter1 +metas: + py_modules: custom_crafter.py # I also tried to add helper.py here \ No newline at end of file diff --git a/tests/integration/issues/github_1546/bad1/custom_crafter.py b/tests/integration/issues/github_1546/bad1/custom_crafter.py new file mode 100644 index 0000000000000..03c6a63745a5f --- /dev/null +++ b/tests/integration/issues/github_1546/bad1/custom_crafter.py @@ -0,0 +1,8 @@ +from jina.executors.crafters import BaseSegmenter +from .helper import helper_function + + +class CustomCrafter1(BaseSegmenter): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + print(helper_function) diff --git a/tests/integration/issues/github_1546/bad1/helper.py b/tests/integration/issues/github_1546/bad1/helper.py new file mode 100644 index 0000000000000..ad02e90ba120f --- /dev/null +++ b/tests/integration/issues/github_1546/bad1/helper.py @@ -0,0 +1,2 @@ +def helper_function(): + pass diff --git a/tests/integration/issues/github_1546/bad2/__init__.py b/tests/integration/issues/github_1546/bad2/__init__.py new file mode 100644 index 0000000000000..f2a3ee73b8f32 --- /dev/null +++ b/tests/integration/issues/github_1546/bad2/__init__.py @@ -0,0 +1,9 @@ +from .helper import helper_function + +from jina.executors.crafters import BaseSegmenter + + +class CustomCrafter2(BaseSegmenter): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + print(helper_function) diff --git a/tests/integration/issues/github_1546/bad2/crafter.yml b/tests/integration/issues/github_1546/bad2/crafter.yml new file mode 100644 index 0000000000000..cb21ec021d2c0 --- /dev/null +++ b/tests/integration/issues/github_1546/bad2/crafter.yml @@ -0,0 +1,5 @@ +!CustomCrafter2 +metas: + py_modules: + - __init__.py + - helper.py diff --git a/tests/integration/issues/github_1546/bad2/helper.py b/tests/integration/issues/github_1546/bad2/helper.py new file mode 100644 index 0000000000000..ad02e90ba120f --- /dev/null +++ b/tests/integration/issues/github_1546/bad2/helper.py @@ -0,0 +1,2 @@ +def helper_function(): + pass diff --git a/tests/integration/issues/github_1546/good1/crafter.yml b/tests/integration/issues/github_1546/good1/crafter.yml new file mode 100644 index 0000000000000..3ba6929c89dbb --- /dev/null +++ b/tests/integration/issues/github_1546/good1/crafter.yml @@ -0,0 +1,5 @@ +!CustomCrafter3 +metas: + py_modules: + - helper.py + - custom_crafter.py \ No newline at end of file diff --git a/tests/integration/issues/github_1546/good1/custom_crafter.py b/tests/integration/issues/github_1546/good1/custom_crafter.py new file mode 100644 index 0000000000000..9535cdf0ac5ee --- /dev/null +++ b/tests/integration/issues/github_1546/good1/custom_crafter.py @@ -0,0 +1,9 @@ +from jinahub.helper import helper_function + +from jina.executors.crafters import BaseSegmenter + + +class CustomCrafter3(BaseSegmenter): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + print(helper_function) diff --git a/tests/integration/issues/github_1546/good1/helper.py b/tests/integration/issues/github_1546/good1/helper.py new file mode 100644 index 0000000000000..ad02e90ba120f --- /dev/null +++ b/tests/integration/issues/github_1546/good1/helper.py @@ -0,0 +1,2 @@ +def helper_function(): + pass diff --git a/tests/integration/issues/github_1546/good2/__init__.py b/tests/integration/issues/github_1546/good2/__init__.py new file mode 100644 index 0000000000000..d12f380d288b9 --- /dev/null +++ b/tests/integration/issues/github_1546/good2/__init__.py @@ -0,0 +1,9 @@ +from .helper import helper_function + +from jina.executors.crafters import BaseSegmenter + + +class CustomCrafter4(BaseSegmenter): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + print(helper_function) diff --git a/tests/integration/issues/github_1546/good2/crafter.yml b/tests/integration/issues/github_1546/good2/crafter.yml new file mode 100644 index 0000000000000..1e5a04d780958 --- /dev/null +++ b/tests/integration/issues/github_1546/good2/crafter.yml @@ -0,0 +1,5 @@ +!CustomCrafter4 +metas: + py_modules: + - helper.py + - __init__.py \ No newline at end of file diff --git a/tests/integration/issues/github_1546/good2/helper.py b/tests/integration/issues/github_1546/good2/helper.py new file mode 100644 index 0000000000000..ad02e90ba120f --- /dev/null +++ b/tests/integration/issues/github_1546/good2/helper.py @@ -0,0 +1,2 @@ +def helper_function(): + pass diff --git a/tests/integration/issues/github_1546/good3/__init__.py b/tests/integration/issues/github_1546/good3/__init__.py new file mode 100644 index 0000000000000..2c008e73a609b --- /dev/null +++ b/tests/integration/issues/github_1546/good3/__init__.py @@ -0,0 +1,60 @@ +from typing import Tuple, Dict, List, Union + +import numpy as np +from jina.executors.crafters import BaseSegmenter + +from .helper import _crop_image, _move_channel_axis, _load_image + + +class FiveImageCropper2(BaseSegmenter): + """ + :class:`FiveImageCropper` crops the image into four corners and the central crop. + """ + + def __init__(self, + target_size: Union[Tuple[int, int], int] = 224, + channel_axis: int = -1, + *args, + **kwargs): + """ + + :param target_size: desired output size. If size is a sequence like (h, w), the output size will be matched to + this. If size is an int, the output will have the same height and width as the `target_size`. + """ + super().__init__(*args, **kwargs) + self.target_size = target_size + self.channel_axis = channel_axis + + def craft(self, blob: 'np.ndarray', *args, **kwargs) -> List[Dict]: + """ + Crop the input image array. + + :param blob: the ndarray of the image with the color channel at the last axis + :return: a list of five chunk dicts with the cropped images + """ + raw_img = _load_image(blob, self.channel_axis) + image_width, image_height = raw_img.size + if isinstance(self.target_size, int): + target_h = target_w = self.target_size + elif isinstance(self.target_size, Tuple) and len(self.target_size) == 2: + target_h, target_w = self.target_size + else: + raise ValueError(f'target_size should be an integer or a tuple of two integers: {self.target_size}') + _tl, top_tl, left_tl = _crop_image(raw_img, self.target_size, 0, 0) + tl = _move_channel_axis(np.asarray(_tl), -1, self.channel_axis) + _tr, top_tr, left_tr = _crop_image(raw_img, self.target_size, top=0, left=image_width - target_w) + tr = _move_channel_axis(np.asarray(_tr), -1, self.channel_axis) + _bl, top_bl, left_bl = _crop_image(raw_img, self.target_size, top=image_height - target_h, left=0) + bl = _move_channel_axis(np.asarray(_bl), -1, self.channel_axis) + _br, top_br, left_br = _crop_image(raw_img, self.target_size, top=image_height - target_h, + left=image_width - target_w) + br = _move_channel_axis(np.asarray(_br), -1, self.channel_axis) + _center, top_center, left_center = _crop_image(raw_img, self.target_size, how='center') + center = _move_channel_axis(np.asarray(_center), -1, self.channel_axis) + return [ + dict(offset=0, weight=1., blob=tl.astype('float32'), location=(top_tl, left_tl)), + dict(offset=0, weight=1., blob=tr.astype('float32'), location=(top_tr, left_tr)), + dict(offset=0, weight=1., blob=bl.astype('float32'), location=(top_bl, left_bl)), + dict(offset=0, weight=1., blob=br.astype('float32'), location=(top_br, left_br)), + dict(offset=0, weight=1., blob=center.astype('float32'), location=(top_center, left_center)), + ] diff --git a/tests/integration/issues/github_1546/good3/config.yml b/tests/integration/issues/github_1546/good3/config.yml new file mode 100644 index 0000000000000..8b3458a8b5ab6 --- /dev/null +++ b/tests/integration/issues/github_1546/good3/config.yml @@ -0,0 +1,8 @@ +!FiveImageCropper2 +with: + {} +metas: + py_modules: + # - you can put more dependencies here + - helper.py + - __init__.py \ No newline at end of file diff --git a/tests/integration/issues/github_1546/good3/helper.py b/tests/integration/issues/github_1546/good3/helper.py new file mode 100644 index 0000000000000..aebf3a9f59efc --- /dev/null +++ b/tests/integration/issues/github_1546/good3/helper.py @@ -0,0 +1,99 @@ +__copyright__ = "Copyright (c) 2020 Jina AI Limited. All rights reserved." +__license__ = "Apache-2.0" + +from typing import Tuple, Union + +import numpy as np + + +def _move_channel_axis(img: 'np.ndarray', channel_axis_to_move: int, target_channel_axis: int = -1) -> 'np.ndarray': + """ + Ensure the color channel axis is the default axis. + """ + if channel_axis_to_move == target_channel_axis: + return img + return np.moveaxis(img, channel_axis_to_move, target_channel_axis) + + +def _load_image(blob: 'np.ndarray', channel_axis: int): + """ + Load an image array and return a `PIL.Image` object. + """ + + from PIL import Image + img = _move_channel_axis(blob, channel_axis) + return Image.fromarray(img.astype('uint8')) + + +def _crop_image(img, target_size: Union[Tuple[int, int], int], top: int = None, left: int = None, how: str = 'precise'): + """ + Crop the input :py:mod:`PIL` image. + + :param img: :py:mod:`PIL.Image`, the image to be resized + :param target_size: desired output size. If size is a sequence like + (h, w), the output size will be matched to this. If size is an int, + the output will have the same height and width as the `target_size`. + :param top: the vertical coordinate of the top left corner of the crop box. + :param left: the horizontal coordinate of the top left corner of the crop box. + :param how: the way of cropping. Valid values include `center`, `random`, and, `precise`. Default is `precise`. + - `center`: crop the center part of the image + - `random`: crop a random part of the image + - `precise`: crop the part of the image specified by the crop box with the given ``top`` and ``left``. + .. warning:: When `precise` is used, ``top`` and ``left`` must be fed valid value. + + """ + import PIL.Image as Image + assert isinstance(img, Image.Image), 'img must be a PIL.Image' + img_w, img_h = img.size + if isinstance(target_size, int): + target_h = target_w = target_size + elif isinstance(target_size, Tuple) and len(target_size) == 2: + target_h, target_w = target_size + else: + raise ValueError(f'target_size should be an integer or a tuple of two integers: {target_size}') + w_beg = left + h_beg = top + if how == 'center': + w_beg = int((img_w - target_w) / 2) + h_beg = int((img_h - target_h) / 2) + elif how == 'random': + w_beg = np.random.randint(0, img_w - target_w + 1) + h_beg = np.random.randint(0, img_h - target_h + 1) + elif how == 'precise': + assert (w_beg is not None and h_beg is not None) + assert (0 <= w_beg <= (img_w - target_w)), f'left must be within [0, {img_w - target_w}]: {w_beg}' + assert (0 <= h_beg <= (img_h - target_h)), f'top must be within [0, {img_h - target_h}]: {h_beg}' + else: + raise ValueError(f'unknown input how: {how}') + if not isinstance(w_beg, int): + raise ValueError(f'left must be int number between 0 and {img_w}: {left}') + if not isinstance(h_beg, int): + raise ValueError(f'top must be int number between 0 and {img_h}: {top}') + w_end = w_beg + target_w + h_end = h_beg + target_h + img = img.crop((w_beg, h_beg, w_end, h_end)) + return img, h_beg, w_beg + + +def _resize_short(img, target_size: Union[Tuple[int, int], int], how: str = 'LANCZOS'): + """ + Resize the input :py:mod:`PIL` image. + :param img: :py:mod:`PIL.Image`, the image to be resized + :param target_size: desired output size. If size is a sequence like (h, w), the output size will be matched to + this. If size is an int, the smaller edge of the image will be matched to this number maintain the aspect + ratio. + :param how: the interpolation method. Valid values include `NEAREST`, `BILINEAR`, `BICUBIC`, and `LANCZOS`. + Default is `LANCZOS`. Please refer to `PIL.Image` for detaisl. + """ + import PIL.Image as Image + assert isinstance(img, Image.Image), 'img must be a PIL.Image' + if isinstance(target_size, int): + percent = float(target_size) / min(img.size[0], img.size[1]) + target_w = int(round(img.size[0] * percent)) + target_h = int(round(img.size[1] * percent)) + elif isinstance(target_size, Tuple) and len(target_size) == 2: + target_h, target_w = target_size + else: + raise ValueError(f'target_size should be an integer or a tuple of two integers: {target_size}') + img = img.resize((target_w, target_h), getattr(Image, how)) + return img diff --git a/tests/integration/issues/github_1546/test_pymodules_import.py b/tests/integration/issues/github_1546/test_pymodules_import.py new file mode 100644 index 0000000000000..4a8d07705f5d5 --- /dev/null +++ b/tests/integration/issues/github_1546/test_pymodules_import.py @@ -0,0 +1,85 @@ +import pytest + +from jina.executors import BaseExecutor + + +def test_import_with_abs_namespace_should_pass(): + """ + This is a valid structure: + - "my_cust_module" is not a python module (lack of __init__.py under the root) + - to import ``foo.py``, you must to use ``from jinahub.foo import bar`` + - ``jinahub`` is a common namespace for all plugin-modules, not changeable. + - ``helper.py`` needs to be put BEFORE `my_cust.py` in YAML ``py_modules`` + + File structure: + + my_cust_module + |- my_cust.py + |- helper.py + |- config.yml + |- py_modules + |- helper.py + |- my_cust.py + """ + + b = BaseExecutor.load_config('good1/crafter.yml') + assert b.__class__.__name__ == 'CustomCrafter3' + + +def test_import_with_module_structure_should_pass(): + """ + This is a valid structure and it is RECOMMENDED: + - "my_cust_module" is a python module + - all core logic of your customized executor goes to ``__init__.py`` + - to import ``foo.py``, you can use relative import, e.g. ``from .foo import bar`` + - ``helper.py`` needs to be put BEFORE `__init__.py` in YAML ``py_modules`` + + This is also the structure given by ``jina hub new`` CLI. + + File structure: + + my_cust_module + |- __init__.py + |- helper.py + |- config.yml + |- py_modules + |- helper.py + |- __init__.py + """ + b = BaseExecutor.load_config('good2/crafter.yml') + assert b.__class__.__name__ == 'CustomCrafter4' + + +def test_import_with_hub_structure_should_pass(): + """ + copy paste from hub module structure should work + this structure is copy-paste from: https://github.com/jina-ai/jina-hub/tree/master/crafters/image/FiveImageCropper + + File structure: + my_cust_modul + | + |- __init__.py + |- helper.py + |- config.yml + |- py_modules + |- helper.py + |- __init.py + :return: + """ + b = BaseExecutor.load_config('good3/config.yml') + assert b.__class__.__name__ == 'FiveImageCropper2' + + +def test_import_casual_structure_should_fail(): + # this structure is a copy-paste from + # https://github.com/jina-ai/jina/issues/1546#issuecomment-751481422 + with pytest.raises(ImportError): + BaseExecutor.load_config('bad1/crafter.yml') + + +def test_import_good_structure_but_wrong_import_order_should_fail(): + # this structure is a copy paste of "test_import_with_module_structure_should_pass" but with wrong import order + # if A depends on B, i.e. in A.py you write "import B" + # then B.py should be put in front of A.py in py_modules, otherwise it will fail + with pytest.raises(ImportError): + BaseExecutor.load_config('bad2/crafter.yml')