From 6ec11e8d1aadcc39f35f147dbe9181258f454c39 Mon Sep 17 00:00:00 2001 From: Zach Burnett Date: Thu, 22 Dec 2022 14:17:34 -0500 Subject: [PATCH 1/9] add specific exception when ellipse construction attempted without `opencv-python` installed --- .github/workflows/ci.yml | 3 +++ pyproject.toml | 2 +- src/stcal/jump/jump.py | 57 +++++++++++++++++++++++++--------------- tox.ini | 2 ++ 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d79d0190..f4615dbc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,6 +73,9 @@ jobs: - toxenv: test-numpy120 os: ubuntu-latest python-version: '3.8' + - toxenv: test-opencv-xdist + os: ubuntu-latest + python-version: '3.x' - toxenv: test-jwst-xdist-cov os: ubuntu-latest python-version: '3.x' diff --git a/pyproject.toml b/pyproject.toml index 069e5bef..16f80e5b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,7 +35,7 @@ test = [ 'pytest-doctestplus', 'pytest-openfiles >=0.5.0', ] -opencv = [ +jump_ellipses = [ 'opencv-python >=4.6.0.66', ] diff --git a/src/stcal/jump/jump.py b/src/stcal/jump/jump.py index 1ab252b2..f2a33216 100644 --- a/src/stcal/jump/jump.py +++ b/src/stcal/jump/jump.py @@ -1,21 +1,20 @@ -import time import logging +import multiprocessing +import time import warnings import numpy as np -from . import twopoint_difference as twopt -from . import constants -import multiprocessing +from . import constants +from . import twopoint_difference as twopt +ELLIPSE_PACKAGE = None try: import cv2 as cv - OPENCV_INSTALLED = True -except ImportError: - OPENCV_INSTALLED = False - warnings.warn('Could not import `opencv-python`; ' - 'certain snowball detection and usage of ellipses will be inoperable') + ELLIPSE_PACKAGE = 'opencv-python' +except (ImportError, ModuleNotFoundError): + ELLIPSE_PACKAGE_WARNING = f'`opencv-python` must be installed to use ellipse construction' log = logging.getLogger(__name__) log.setLevel(logging.DEBUG) @@ -237,7 +236,7 @@ def detect_jumps(frames_per_group, data, gdq, pdq, err, # modified unless copied beforehand gdq = gdq.copy() data = data.copy() - copy_arrs = False # we dont need to copy arrays again in find_crs + copy_arrs = False # we dont need to copy arrays again in find_crs for i in range(n_slices - 1): slices.insert(i, (data[:, :, i * yinc:(i + 1) * yinc, :], @@ -398,8 +397,13 @@ def extend_snowballs(plane, snowballs, sat_flag, jump_flag, expansion=1.5): jump_center = snowball[0] cenx = jump_center[1] ceny = jump_center[0] + center = (round(ceny), round(cenx)) extend_radius = round(jump_radius * expansion) - image = cv.circle(image, (round(ceny), round(cenx)), extend_radius, (0, 0, 4), -1) + color = (0, 0, 4) + if ELLIPSE_PACKAGE == 'opencv-python': + image = cv.circle(image, center, extend_radius, color, -1) + else: + raise ModuleNotFoundError(ELLIPSE_PACKAGE_WARNING) jump_circle = image[:, :, 2] saty, satx = np.where(sat_pix == 2) jump_circle[saty, satx] = 0 @@ -428,8 +432,13 @@ def extend_ellipses(plane, ellipses, sat_flag, jump_flag, expansion=1.1): axis1 = ellipse[1][0] * expansion axis2 = ellipse[1][1] + (expansion - 1.0) * ellipse[1][0] alpha = ellipse[2] - image = cv.ellipse(image, (round(ceny), round(cenx)), (round(axis1 / 2), - round(axis2 / 2)), alpha, 0, 360, (0, 0, 4), -1) + center = (round(ceny), round(cenx)) + axes = (round(axis1 / 2), round(axis2 / 2)) + color = (0, 0, 4) + if ELLIPSE_PACKAGE == 'opencv-python': + image = cv.ellipse(image, center, axes, alpha, 0, 360, color, -1) + else: + raise ModuleNotFoundError(ELLIPSE_PACKAGE_WARNING) jump_ellipse = image[:, :, 2] saty, satx = np.where(sat_pix == 2) jump_ellipse[saty, satx] = 0 @@ -441,9 +450,12 @@ def find_circles(dqplane, bitmask, min_area): # Using an input DQ plane this routine will find the groups of pixels with at least the minimum # area and return a list of the minimum enclosing circle parameters. pixels = np.bitwise_and(dqplane, bitmask) - contours, hierarchy = cv.findContours(pixels, cv.RETR_EXTERNAL, cv.CHAIN_APPROX_SIMPLE) - bigcontours = [con for con in contours if cv.contourArea(con) >= min_area] - circles = [cv.minEnclosingCircle(con) for con in bigcontours] + if ELLIPSE_PACKAGE == 'opencv-python': + contours, hierarchy = cv.findContours(pixels, cv.RETR_EXTERNAL, cv.CHAIN_APPROX_SIMPLE) + bigcontours = [con for con in contours if cv.contourArea(con) >= min_area] + circles = [cv.minEnclosingCircle(con) for con in bigcontours] + else: + raise ModuleNotFoundError(ELLIPSE_PACKAGE_WARNING) return circles @@ -451,11 +463,14 @@ def find_ellipses(dqplane, bitmask, min_area): # Using an input DQ plane this routine will find the groups of pixels with at least the minimum # area and return a list of the minimum enclosing ellipse parameters. pixels = np.bitwise_and(dqplane, bitmask) - contours, hierarchy = cv.findContours(pixels, cv.RETR_EXTERNAL, cv.CHAIN_APPROX_SIMPLE) - bigcontours = [con for con in contours if cv.contourArea(con) > min_area] - # minAreaRect is used becuase fitEllipse requires 5 points and it is possible to have a contour - # with just 4 points. - ellipses = [cv.minAreaRect(con) for con in bigcontours] + if ELLIPSE_PACKAGE == 'opencv-python': + contours, hierarchy = cv.findContours(pixels, cv.RETR_EXTERNAL, cv.CHAIN_APPROX_SIMPLE) + bigcontours = [con for con in contours if cv.contourArea(con) > min_area] + # minAreaRect is used becuase fitEllipse requires 5 points and it is possible to have a contour + # with just 4 points. + ellipses = [cv.minAreaRect(con) for con in bigcontours] + else: + raise ModuleNotFoundError(ELLIPSE_PACKAGE_WARNING) return ellipses diff --git a/tox.ini b/tox.ini index 119f8010..da229576 100644 --- a/tox.ini +++ b/tox.ini @@ -51,6 +51,7 @@ description = xdist: using parallel processing extras = test + jump_ellipses: jump_ellipses deps = xdist: pytest-xdist jwst: jwst[test] @ git+https://github.com/spacetelescope/jwst.git @@ -73,6 +74,7 @@ commands = jwst: --pyargs jwst --ignore-glob=timeconversion --ignore-glob=associations \ romancal: --pyargs romancal \ cov: --cov=. --cov-config=pyproject.toml --cov-report=term-missing --cov-report=xml \ + jump_ellipses: -- tests/test_jump.py \ {posargs} [testenv:build-docs] From 739dcd612283b91950a2c54468cc1cb37f8dcaff Mon Sep 17 00:00:00 2001 From: Zach Burnett Date: Thu, 22 Dec 2022 14:20:31 -0500 Subject: [PATCH 2/9] add installation instructions --- src/stcal/jump/jump.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/stcal/jump/jump.py b/src/stcal/jump/jump.py index f2a33216..6c138442 100644 --- a/src/stcal/jump/jump.py +++ b/src/stcal/jump/jump.py @@ -1,7 +1,6 @@ import logging import multiprocessing import time -import warnings import numpy as np @@ -14,7 +13,7 @@ ELLIPSE_PACKAGE = 'opencv-python' except (ImportError, ModuleNotFoundError): - ELLIPSE_PACKAGE_WARNING = f'`opencv-python` must be installed to use ellipse construction' + ELLIPSE_PACKAGE_WARNING = f'`opencv-python` must be installed (`pip install jwst[jump_ellipses]`) to use ellipses' log = logging.getLogger(__name__) log.setLevel(logging.DEBUG) From ccfe340eeda155c1b7fb40e3d8fd2d7370f28ddb Mon Sep 17 00:00:00 2001 From: Zach Burnett Date: Thu, 22 Dec 2022 14:22:00 -0500 Subject: [PATCH 3/9] add change log entry --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index b9b9ecd8..0fe9f10e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -9,7 +9,7 @@ General Bug Fixes --------- -- +- improve exception handling when attempting to use ellipses without ``opencv-python`` installed [#136] Changes to API -------------- From bbbcbff015d1bd1a35446fb0c4a653214907ec74 Mon Sep 17 00:00:00 2001 From: Zach Burnett Date: Thu, 22 Dec 2022 14:23:43 -0500 Subject: [PATCH 4/9] formatting --- src/stcal/jump/jump.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/stcal/jump/jump.py b/src/stcal/jump/jump.py index 6c138442..19684e6a 100644 --- a/src/stcal/jump/jump.py +++ b/src/stcal/jump/jump.py @@ -13,7 +13,8 @@ ELLIPSE_PACKAGE = 'opencv-python' except (ImportError, ModuleNotFoundError): - ELLIPSE_PACKAGE_WARNING = f'`opencv-python` must be installed (`pip install jwst[jump_ellipses]`) to use ellipses' + ELLIPSE_PACKAGE_WARNING = '`opencv-python` must be installed (`pip install jwst[jump_ellipses]`) ' \ + 'in order to use ellipses' log = logging.getLogger(__name__) log.setLevel(logging.DEBUG) From 9ec522fba954c0fbf86e016c0358c38de4944944 Mon Sep 17 00:00:00 2001 From: Zach Burnett Date: Thu, 22 Dec 2022 14:26:00 -0500 Subject: [PATCH 5/9] simplify naming --- pyproject.toml | 2 +- tox.ini | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 16f80e5b..069e5bef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,7 +35,7 @@ test = [ 'pytest-doctestplus', 'pytest-openfiles >=0.5.0', ] -jump_ellipses = [ +opencv = [ 'opencv-python >=4.6.0.66', ] diff --git a/tox.ini b/tox.ini index da229576..fa2928da 100644 --- a/tox.ini +++ b/tox.ini @@ -51,7 +51,7 @@ description = xdist: using parallel processing extras = test - jump_ellipses: jump_ellipses + opencv: opencv deps = xdist: pytest-xdist jwst: jwst[test] @ git+https://github.com/spacetelescope/jwst.git @@ -74,7 +74,7 @@ commands = jwst: --pyargs jwst --ignore-glob=timeconversion --ignore-glob=associations \ romancal: --pyargs romancal \ cov: --cov=. --cov-config=pyproject.toml --cov-report=term-missing --cov-report=xml \ - jump_ellipses: -- tests/test_jump.py \ + opencv: -- tests/test_jump.py \ {posargs} [testenv:build-docs] From 26cdf92ecfb606453b64184fd6fb9795c15d9b22 Mon Sep 17 00:00:00 2001 From: Zach Burnett Date: Thu, 22 Dec 2022 14:28:52 -0500 Subject: [PATCH 6/9] xfail opencv tests --- tests/test_jump.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_jump.py b/tests/test_jump.py index af69e080..f797cd3c 100644 --- a/tests/test_jump.py +++ b/tests/test_jump.py @@ -31,7 +31,7 @@ def _cube(ngroups, readnoise=10): return _cube -@pytest.mark.skipif(not OPENCV_INSTALLED, reason="`opencv-python` not installed") +@pytest.mark.xfail(not OPENCV_INSTALLED, reason="`opencv-python` not installed") def test_find_simple_circle(): plane = np.zeros(shape=(5, 5), dtype=np.uint8) plane[2, 2] = DQFLAGS['JUMP_DET'] @@ -43,7 +43,7 @@ def test_find_simple_circle(): assert circle[0][1] == pytest.approx(1.0, 1e-3) -@pytest.mark.skipif(not OPENCV_INSTALLED, reason="`opencv-python` not installed") +@pytest.mark.xfail(not OPENCV_INSTALLED, reason="`opencv-python` not installed") def test_find_simple_ellipse(): plane = np.zeros(shape=(5, 5), dtype=np.uint8) plane[2, 2] = DQFLAGS['JUMP_DET'] From 702f6e37cac2472153a714cde46f2e7c562ebe1a Mon Sep 17 00:00:00 2001 From: Zach Burnett Date: Thu, 22 Dec 2022 14:46:19 -0500 Subject: [PATCH 7/9] add toxenv description --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index fa2928da..9f0ac349 100644 --- a/tox.ini +++ b/tox.ini @@ -46,6 +46,7 @@ description = run tests jwst: of JWST pipeline romancal: of Romancal pipeline + opencv: requiring opencv-python warnings: treating warnings as errors cov: with coverage xdist: using parallel processing From b091c68f86139cc02ef240bfaae015afeef30a74 Mon Sep 17 00:00:00 2001 From: Zach Burnett Date: Thu, 22 Dec 2022 14:51:19 -0500 Subject: [PATCH 8/9] fix installation instructions --- src/stcal/jump/jump.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/stcal/jump/jump.py b/src/stcal/jump/jump.py index 19684e6a..f5a4dc30 100644 --- a/src/stcal/jump/jump.py +++ b/src/stcal/jump/jump.py @@ -13,8 +13,7 @@ ELLIPSE_PACKAGE = 'opencv-python' except (ImportError, ModuleNotFoundError): - ELLIPSE_PACKAGE_WARNING = '`opencv-python` must be installed (`pip install jwst[jump_ellipses]`) ' \ - 'in order to use ellipses' + ELLIPSE_PACKAGE_WARNING = '`opencv-python` must be installed (`pip install stcal[opencv]`) in order to use ellipses' log = logging.getLogger(__name__) log.setLevel(logging.DEBUG) From 1dbb1af852b50a81cf4bf5066c824c3ce18e5dbd Mon Sep 17 00:00:00 2001 From: Zach Burnett Date: Thu, 22 Dec 2022 15:00:30 -0500 Subject: [PATCH 9/9] formatting --- src/stcal/jump/jump.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/stcal/jump/jump.py b/src/stcal/jump/jump.py index f5a4dc30..5ed1ca43 100644 --- a/src/stcal/jump/jump.py +++ b/src/stcal/jump/jump.py @@ -13,7 +13,8 @@ ELLIPSE_PACKAGE = 'opencv-python' except (ImportError, ModuleNotFoundError): - ELLIPSE_PACKAGE_WARNING = '`opencv-python` must be installed (`pip install stcal[opencv]`) in order to use ellipses' + ELLIPSE_PACKAGE_WARNING = '`opencv-python` must be installed (`pip install stcal[opencv]`) ' \ + 'in order to use ellipses' log = logging.getLogger(__name__) log.setLevel(logging.DEBUG)