Skip to content

Commit

Permalink
refactor: Make warnings error in tests (#1192)
Browse files Browse the repository at this point in the history
Change `pytest` configuration to change warnings into errors in the
tests.

Fix code that emites warning.

<!-- Generated by sourcery-ai[bot]: start summary -->

## Summary by Sourcery

Convert warnings to errors in tests by updating the `pytest`
configuration and refactor code to handle warnings appropriately.
Enhance the `sentry_sdk` usage by replacing `push_scope` with
`new_scope`. Improve data handling in `batch_backend.py` and update
tests to align with these changes.

Bug Fixes:
- Fix the `test_no_update` test to correctly handle URL errors by using
`urllib.error.URLError`.
- Correct the `test_ignore_file_exists_old_date` test to assert the
correct release version and URL.

Enhancements:
- Update the `sentry_sdk` usage to replace `push_scope` with `new_scope`
for better scope management.
- Improve the `CheckVersionThread` initialization by updating the
default URL to a more relevant one.
- Enhance the `get_data_to_write` method in `batch_backend.py` to handle
empty data frames more efficiently.
- Refactor the `decode_zstd1` and `decode_zstd0` functions to use
`np.frombuffer` instead of `np.fromstring` for better performance and
compatibility.

Tests:
- Modify tests to treat warnings as errors by updating the `pytest`
configuration.
- Update tests to reflect changes in the `sentry_sdk` scope management.

<!-- Generated by sourcery-ai[bot]: end summary -->

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

## Release Notes

- **New Features**
- Enhanced handling of color information in ROI parameters, allowing for
greater flexibility with color keys.
- Improved compatibility with newer versions of `napari` by adjusting
colormap settings.

- **Bug Fixes**
- Ensured default color values are returned correctly when no regions of
interest are present.

- **Documentation**
	- Updated type annotations for better clarity and usability.

- **Tests**
- Enhanced test cases related to color handling and ROI parameters to
improve robustness.
- Added version checks for `napari` to ensure correct behavior based on
library version.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
  • Loading branch information
Czaki and sourcery-ai[bot] authored Sep 20, 2024
1 parent a7ab81f commit 48f82c1
Show file tree
Hide file tree
Showing 16 changed files with 105 additions and 62 deletions.
6 changes: 2 additions & 4 deletions package/PartSeg/_launcher/check_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ class CheckVersionThread(QThread):
.. _PYPI: https://pypi.org/project/PartSeg/
"""

def __init__(
self, package_name="PartSeg", default_url="https://4dnucleome.cent.uw.edu.pl/PartSeg/", base_version=__version__
):
def __init__(self, package_name="PartSeg", default_url="https://partseg.github.io/", base_version=__version__):
super().__init__()
self.release = base_version
self.base_release = base_version
Expand Down Expand Up @@ -57,7 +55,7 @@ def run(self):
except (KeyError, urllib.error.URLError): # pragma: no cover
pass
except Exception as e: # pylint: disable=broad-except
with sentry_sdk.push_scope() as scope:
with sentry_sdk.new_scope() as scope:
scope.set_tag("auto_report", "true")
scope.set_tag("check_version", "true")
sentry_sdk.capture_exception(e)
Expand Down
2 changes: 1 addition & 1 deletion package/PartSeg/common_backend/except_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def my_excepthook(type_, value, trace_back):
# log the exception here
if state_store.show_error_dialog and not isinstance(value, KeyboardInterrupt):
if state_store.auto_report or state_store.always_report:
with sentry_sdk.push_scope() as scope:
with sentry_sdk.new_scope() as scope:
scope.set_tag("auto_report", "true")
scope.set_tag("main_thread", QCoreApplication.instance().thread() == QThread.currentThread())
sentry_sdk.capture_exception(value)
Expand Down
4 changes: 2 additions & 2 deletions package/PartSeg/common_gui/error_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ def send_report(self):
"""
Function with construct final error message and send it using sentry.
"""
with sentry_sdk.push_scope() as scope:
with sentry_sdk.new_scope() as scope:
text = f"{self.desc.text()}\n\nVersion: {__version__}\n"
if len(self.additional_notes) > 0:
scope.set_extra("additional_notes", self.additional_notes)
Expand All @@ -217,7 +217,7 @@ def send_report(self):

event_id = sentry_sdk.capture_event(event, hint=hint)
if event_id is None:
event_id = sentry_sdk.hub.Hub.current.last_event_id()
event_id = sentry_sdk.scope.Scope.last_event_id()

if len(self.additional_info.toPlainText()) > 0:
contact_text = self.contact_info.text()
Expand Down
24 changes: 16 additions & 8 deletions package/PartSeg/common_gui/napari_image_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
from napari._qt.widgets.qt_viewer_buttons import QtViewerPushButton as QtViewerPushButton_
_napari_ge_4_13 = parse_version(napari.__version__) >= parse_version("0.4.13a1")
_napari_ge_4_17 = parse_version(napari.__version__) >= parse_version("0.4.17a1")
_napari_ge_4_19 = parse_version(napari.__version__) >= parse_version("0.4.19")
_napari_ge_5 = parse_version(napari.__version__) >= parse_version("0.5.0a1")

# if run with numpy<2 on macOS arm64 architecture compiled from pypi wheels
Expand Down Expand Up @@ -112,7 +113,7 @@ def __init__(self, viewer):
ORDER_DICT = {"xy": [0, 1, 2, 3], "zy": [0, 2, 1, 3], "zx": [0, 3, 1, 2]}
NEXT_ORDER = {"xy": "zy", "zy": "zx", "zx": "xy"}

ColorInfo = Dict[int, Union[str, List[float]]]
ColorInfo = Dict[Optional[int], Union[str, List[float]]]


@dataclass
Expand Down Expand Up @@ -480,15 +481,18 @@ def get_roi_view_parameters(self, image_info: ImageInfo) -> ColorInfo:
or image_info.roi_count == 0
or colors.size == 0
):
return {x: [0, 0, 0, 0] for x in range(image_info.roi_count + 1)}
return {0: [0, 0, 0, 0], None: [0, 0, 0, 0]}

res = {x: colors[(x - 1) % colors.shape[0]] for x in range(1, image_info.roi_count + 1)}
res[0] = [0, 0, 0, 0]
res[None] = [0, 0, 0, 0]
return res

def set_roi_colormap(self, image_info) -> None:
if _napari_ge_5:
image_info.roi.colormap = self.get_roi_view_parameters(image_info)
if _napari_ge_4_19:
from napari.utils.colormaps import DirectLabelColormap

image_info.roi.colormap = DirectLabelColormap(color_dict=self.get_roi_view_parameters(image_info))
return
if _napari_ge_4_13:
image_info.roi.color = self.get_roi_view_parameters(image_info)
Expand Down Expand Up @@ -591,8 +595,10 @@ def set_mask(self, mask: Optional[np.ndarray] = None, image: Optional[Image] = N
else:
image_info.mask.data = mask_marker
image_info.mask.metadata["valid"] = True
if _napari_ge_5:
image_info.mask.colormap = self.mask_color()
if _napari_ge_4_19:
from napari.utils.colormaps import DirectLabelColormap

image_info.mask.colormap = DirectLabelColormap(color_dict=self.mask_color())
else:
image_info.mask.color = self.mask_color()
image_info.mask.opacity = self.mask_opacity()
Expand All @@ -613,8 +619,10 @@ def update_mask_parameters(self):
for image_info in self.image_info.values():
if image_info.mask is not None:
image_info.mask.opacity = opacity
if _napari_ge_5:
image_info.mask.colormap = colormap
if _napari_ge_4_19:
from napari.utils.colormaps import DirectLabelColormap

image_info.mask.colormap = DirectLabelColormap(color_dict=colormap)
else:
image_info.mask.color = colormap

Expand Down
7 changes: 5 additions & 2 deletions package/PartSeg/plugins/napari_widgets/lables_control.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from PartSeg.plugins.napari_widgets._settings import get_settings

NAPARI_GE_5_0 = parse_version(version("napari")) >= parse_version("0.5.0a1")
NAPARI_GE_4_19 = parse_version(version("napari")) >= parse_version("0.4.19a1")


class NapariLabelShow(LabelShow):
Expand Down Expand Up @@ -41,8 +42,10 @@ def apply_label(self):
max_val = layer.data.max()
labels = {i + 1: [x / 255 for x in self.label[i % len(self.label)]] for i in range(max_val + 5)}
labels[None] = [0, 0, 0, 0]
if NAPARI_GE_5_0:
layer.colormap = labels
if NAPARI_GE_4_19:
from napari.utils.colormaps import DirectLabelColormap

layer.colormap = DirectLabelColormap(color_dict=labels)
else:
layer.color = labels

Expand Down
3 changes: 3 additions & 0 deletions package/PartSeg/plugins/napari_widgets/search_label_widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ def _shift_if_need(self, labels, bound_info):
u_bound = upper_bound[-2:][::-1]
with warnings.catch_warnings():
warnings.filterwarnings("ignore", "Public access to Window.qt_viewer")
warnings.filterwarnings("ignore", "Access to QtViewer.view is deprecated")
rect = Rect(self.napari_viewer.window.qt_viewer.view.camera.get_state()["rect"])
if rect.contains(*l_bound) and rect.contains(*u_bound):
return
Expand All @@ -123,6 +124,7 @@ def _shift_if_need(self, labels, bound_info):
rect.pos = pos
with warnings.catch_warnings():
warnings.filterwarnings("ignore", "Public access to Window.qt_viewer")
warnings.filterwarnings("ignore", "Access to QtViewer.view is deprecated")
self.napari_viewer.window.qt_viewer.view.camera.set_state({"rect": rect})

@staticmethod
Expand All @@ -146,6 +148,7 @@ def _zoom(self):
rect = Rect(pos=(lower_bound - frame)[-2:][::-1], size=(diff + 2 * frame)[-2:][::-1])
with warnings.catch_warnings():
warnings.filterwarnings("ignore", "Public access to Window.qt_viewer")
warnings.filterwarnings("ignore", "Access to QtViewer.view is deprecated")
self.napari_viewer.window.qt_viewer.view.camera.set_state({"rect": rect})
self._update_point(lower_bound, upper_bound)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -602,8 +602,11 @@ def get_data_to_write(self) -> tuple[str, pd.DataFrame]:
"""
sorted_row = [x[1] for x in sorted(self.row_list)]
df = pd.DataFrame(sorted_row, columns=self.columns)
df2 = pd.concat((self.data_frame, df), axis=0)
self.data_frame = df2.reset_index(drop=True)
if self.data_frame.empty:
self.data_frame = df.reset_index(drop=True)
else:
df2 = pd.concat((self.data_frame, df), axis=0)
self.data_frame = df2.reset_index(drop=True)
self.row_list = []
return self.name, self.data_frame

Expand Down Expand Up @@ -840,7 +843,7 @@ def write_to_excel(

if errors:
errors_data = pd.DataFrame(errors, columns=["File path", "error description"])
errors_data.to_excel(writer, "Errors")
errors_data.to_excel(writer, sheet_name="Errors")

@staticmethod
def write_calculation_plan(writer: pd.ExcelWriter, calculation_plan: CalculationPlan):
Expand Down
12 changes: 8 additions & 4 deletions package/PartSegImage/image_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ def decode_zstd1(data: bytes) -> np.ndarray:
header = parse_zstd1_header(data, len(data))
dtype = _get_dtype()
if header.hiLoByteUnpackPreprocessing:
array_ = np.fromstring(imagecodecs.zstd_decode(data[header.header_size :]), np.uint8)
array_ = np.frombuffer(imagecodecs.zstd_decode(data[header.header_size :]), np.uint8).copy()
half_size = array_.size // 2
array = np.empty(half_size, np.uint16)
array[:] = array_[:half_size] + (array_[half_size:].astype(np.uint16) << 8)
array = array.view(dtype)
else:
array = np.fromstring(imagecodecs.zstd_decode(data[header.header_size :]), dtype)
array = np.frombuffer(imagecodecs.zstd_decode(data[header.header_size :]), dtype).copy()
return array


Expand All @@ -86,7 +86,7 @@ def decode_zstd0(data: bytes) -> np.ndarray:
Decode ZSTD0 data
"""
dtype = _get_dtype()
return np.fromstring(imagecodecs.zstd_decode(data), dtype)
return np.frombuffer(imagecodecs.zstd_decode(data), dtype).copy()


if parse_version(version("czifile")) == parse_version("2019.7.2"):
Expand Down Expand Up @@ -306,7 +306,10 @@ class OifImagReader(BaseImageReader):
def read(self, image_path: typing.Union[str, Path], mask_path=None, ext=None) -> Image:
with OifFile(image_path) as image_file:
tiffs = tifffile.natural_sorted(image_file.glob("*.tif"))
with tifffile.TiffFile(image_file.open_file(tiffs[0]), name=tiffs[0]) as tif_file:

with image_file.open_file(tiffs[0]) as tiff_buffer, tifffile.TiffFile(
tiff_buffer, name=tiffs[0]
) as tif_file:
axes = image_file.series[0].axes + tif_file.series[0].axes
image_data = image_file.asarray()
image_data = self.update_array_shape(image_data, axes)
Expand Down Expand Up @@ -366,6 +369,7 @@ def read(self, image_path: typing.Union[str, BytesIO, Path], mask_path=None, ext
# TODO add mask reading
if isinstance(image_path, BytesIO):
image_path = ""
image_file.close()
return self.image_class(
image_data,
self.spacing,
Expand Down
17 changes: 11 additions & 6 deletions package/tests/test_PartSeg/test_check_release.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import urllib.error
import urllib.request
from datetime import date, timedelta
from io import StringIO
Expand Down Expand Up @@ -68,15 +69,17 @@ def exec_():
assert "You can update it from pypi" in values[1]


def test_no_update(monkeypatch, qtbot):
monkeypatch.setattr("PartSeg.state_store.save_folder", False)
def test_no_update(monkeypatch, qtbot, tmp_path):
# check if nothing is reported on ulr error
monkeypatch.setattr("PartSeg.state_store.save_folder", tmp_path)

def urlopen_mock(url):
raise RuntimeError
raise urllib.error.URLError("test purpose")

monkeypatch.setattr(urllib.request, "urlopen", urlopen_mock)
chk_thr = check_version.CheckVersionThread()
chk_thr = check_version.CheckVersionThread(base_version="0.10.0")
chk_thr.run()
assert chk_thr.release == "0.10.0"


def test_error_report(monkeypatch, qtbot):
Expand Down Expand Up @@ -118,13 +121,15 @@ def test_ignore_file_exists_old_date(monkeypatch, qtbot, tmp_path):
monkeypatch.setattr(state_store, "save_folder", tmp_path)

def urlopen_mock(_url):
return StringIO("")
return StringIO('{"info": {"version": "0.11.0", "home_page": "example.org"}}')

monkeypatch.setattr(urllib.request, "urlopen", urlopen_mock)

chk_thr = check_version.CheckVersionThread()
chk_thr = check_version.CheckVersionThread(base_version="0.10.0")
chk_thr.run()
assert not (tmp_path / IGNORE_FILE).exists()
assert chk_thr.release == "0.11.0"
assert chk_thr.url == "example.org"


def test_create_ignore(qtbot, tmp_path, monkeypatch):
Expand Down
4 changes: 2 additions & 2 deletions package/tests/test_PartSeg/test_common_gui.py
Original file line number Diff line number Diff line change
Expand Up @@ -1767,7 +1767,7 @@ def test_create_issue(self, mock_web, qtbot):
assert "body=This" in mock_web.call_args.args[0]

@patch("requests.post")
@patch("sentry_sdk.push_scope")
@patch("sentry_sdk.new_scope")
def test_send_report(self, sentry_mock, request_mock, qtbot):
dialog = ErrorDialog(ValueError("aaa"), "Test text")
qtbot.addWidget(dialog)
Expand All @@ -1777,7 +1777,7 @@ def test_send_report(self, sentry_mock, request_mock, qtbot):
request_mock.assert_not_called()

@patch("requests.post")
@patch("sentry_sdk.push_scope")
@patch("sentry_sdk.new_scope")
@pytest.mark.parametrize(
("email", "expected", "return_code", "post_call_count"),
[
Expand Down
6 changes: 5 additions & 1 deletion package/tests/test_PartSeg/test_napari_image_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,20 @@
from PartSegImage import Image

NAPARI_GE_5_0 = parse_version(version("napari")) >= parse_version("0.5.0a1")
NAPARI_GE_4_19 = parse_version(version("napari")) >= parse_version("0.4.19a1")


if NAPARI_GE_5_0:
EXPECTED_RANGE = (0, 0, 1)
else:
EXPECTED_RANGE = (0, 1, 1)

if NAPARI_GE_4_19:

def get_color_dict(layer):
return layer.colormap.color_dict

else:
EXPECTED_RANGE = (0, 1, 1)

def get_color_dict(layer):
return layer.color
Expand Down
6 changes: 4 additions & 2 deletions package/tests/test_PartSeg/test_napari_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@
from PartSegCore.segmentation.watershed import WatershedSelection

NAPARI_GE_5_0 = parse_version(version("napari")) >= parse_version("0.5.0a1")
NAPARI_GE_4_19 = parse_version(version("napari")) >= parse_version("0.4.19a1")

if NAPARI_GE_5_0:
if NAPARI_GE_4_19:

def check_auto_mode(layer):
from napari.utils.colormaps import CyclicLabelColormap
Expand Down Expand Up @@ -387,7 +388,8 @@ def test_napari_label_show(viewer_with_data, qtbot):
viewer_with_data.layers.selection.remove(viewer_with_data.layers["image"])
assert widget.apply_label_btn.isEnabled()
check_auto_mode(viewer_with_data.layers["label"])
widget.apply_label_btn.click()
with qtbot.waitSignal(widget.apply_label_btn.clicked):
widget.apply_label_btn.click()
check_direct_mode(viewer_with_data.layers["label"])


Expand Down
Loading

0 comments on commit 48f82c1

Please sign in to comment.