From 240007eb7ae827ca76415046bcaa80a8027708da Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Thu, 30 Oct 2025 01:13:23 -0700 Subject: [PATCH 1/4] misc cleanup: variable naming, typing, comments --- masque/error.py | 4 +++- masque/ports.py | 22 +++++++++++----------- masque/traits/positionable.py | 2 +- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/masque/error.py b/masque/error.py index 0e46849..00775d3 100644 --- a/masque/error.py +++ b/masque/error.py @@ -25,12 +25,14 @@ class BuildError(MasqueError): """ pass + class PortError(MasqueError): """ - Exception raised by builder-related functions + Exception raised by port-related functions """ pass + class OneShotError(MasqueError): """ Exception raised when a function decorated with `@oneshot` is called more than once diff --git a/masque/ports.py b/masque/ports.py index 8060616..f12706e 100644 --- a/masque/ports.py +++ b/masque/ports.py @@ -305,11 +305,11 @@ class PortList(metaclass=ABCMeta): if type_conflicts.any(): msg = 'Ports have conflicting types:\n' - for nn, (k, v) in enumerate(connections.items()): + for nn, (kk, vv) in enumerate(connections.items()): if type_conflicts[nn]: - msg += f'{k} | {a_types[nn]}:{b_types[nn]} | {v}\n' msg = ''.join(traceback.format_stack()) + '\n' + msg warnings.warn(msg, stacklevel=2) + msg += f'{kk} | {a_types[nn]}:{b_types[nn]} | {vv}\n' a_offsets = numpy.array([pp.offset for pp in a_ports]) b_offsets = numpy.array([pp.offset for pp in b_ports]) @@ -326,17 +326,17 @@ class PortList(metaclass=ABCMeta): if not numpy.allclose(rotations, 0): rot_deg = numpy.rad2deg(rotations) msg = 'Port orientations do not match:\n' - for nn, (k, v) in enumerate(connections.items()): + for nn, (kk, vv) in enumerate(connections.items()): if not numpy.isclose(rot_deg[nn], 0): - msg += f'{k} | {rot_deg[nn]:g} | {v}\n' + msg += f'{kk} | {rot_deg[nn]:g} | {vv}\n' raise PortError(msg) translations = a_offsets - b_offsets if not numpy.allclose(translations, 0): msg = 'Port translations do not match:\n' - for nn, (k, v) in enumerate(connections.items()): + for nn, (kk, vv) in enumerate(connections.items()): if not numpy.allclose(translations[nn], 0): - msg += f'{k} | {translations[nn]} | {v}\n' + msg += f'{kk} | {translations[nn]} | {vv}\n' raise PortError(msg) for pp in chain(a_names, b_names): @@ -406,7 +406,7 @@ class PortList(metaclass=ABCMeta): map_out_counts = Counter(map_out.values()) map_out_counts[None] = 0 - conflicts_out = {k for k, v in map_out_counts.items() if v > 1} + conflicts_out = {kk for kk, vv in map_out_counts.items() if vv > 1} if conflicts_out: raise PortError(f'Duplicate targets in `map_out`: {conflicts_out}') @@ -438,7 +438,7 @@ class PortList(metaclass=ABCMeta): `set_rotation` must remain `None`. ok_connections: Set of "allowed" ptype combinations. Identical ptypes are always allowed to connect, as is `'unk'` with - any other ptypte. Non-allowed ptype connections will emit a + any other ptypte. Non-allowed ptype connections will log a warning. Order is ignored, i.e. `(a, b)` is equivalent to `(b, a)`. @@ -489,7 +489,7 @@ class PortList(metaclass=ABCMeta): `set_rotation` must remain `None`. ok_connections: Set of "allowed" ptype combinations. Identical ptypes are always allowed to connect, as is `'unk'` with - any other ptypte. Non-allowed ptype connections will emit a + any other ptypte. Non-allowed ptype connections will log a warning. Order is ignored, i.e. `(a, b)` is equivalent to `(b, a)`. @@ -520,11 +520,11 @@ class PortList(metaclass=ABCMeta): for st, ot in zip(s_types, o_types, strict=True)]) if type_conflicts.any(): msg = 'Ports have conflicting types:\n' - for nn, (k, v) in enumerate(map_in.items()): + for nn, (kk, vv) in enumerate(map_in.items()): if type_conflicts[nn]: - msg += f'{k} | {s_types[nn]}:{o_types[nn]} | {v}\n' msg = ''.join(traceback.format_stack()) + '\n' + msg warnings.warn(msg, stacklevel=2) + msg += f'{kk} | {s_types[nn]}:{o_types[nn]} | {vv}\n' rotations = numpy.mod(s_rotations - o_rotations - pi, 2 * pi) if not has_rot.any(): diff --git a/masque/traits/positionable.py b/masque/traits/positionable.py index fd8551b..6779869 100644 --- a/masque/traits/positionable.py +++ b/masque/traits/positionable.py @@ -1,4 +1,4 @@ -from typing import Self, Any +from typing import Self from abc import ABCMeta, abstractmethod import numpy From dadaf48d35a42d2c98480b6214aee34519ab56a5 Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Thu, 30 Oct 2025 01:14:37 -0700 Subject: [PATCH 2/4] [error, ports] Make stack traces more directly reflect teh location of the issue --- masque/error.py | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ masque/ports.py | 12 +++++------ 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/masque/error.py b/masque/error.py index 00775d3..95c3fd2 100644 --- a/masque/error.py +++ b/masque/error.py @@ -1,3 +1,10 @@ +import traceback +import pathlib + + +MASQUE_DIR = str(pathlib.Path(__file__).parent) + + class MasqueError(Exception): """ Parent exception for all Masque-related Exceptions @@ -39,3 +46,50 @@ class OneShotError(MasqueError): """ def __init__(self, func_name: str) -> None: Exception.__init__(self, f'Function "{func_name}" with @oneshot was called more than once') + + +def format_stacktrace( + stacklevel: int = 1, + *, + skip_file_prefixes: tuple[str, ...] = (MASQUE_DIR,), + low_file_prefixes: tuple[str, ...] = (''), + low_file_suffixes: tuple[str, ...] = ('IPython/utils/py3compat.py', ), + ) -> str: + """ + Utility function for making nicer stack traces (e.g. excluding and similar) + + Args: + stacklevel: Number of frames to remove from near this function (default is to + show caller but not ourselves). Similar to `warnings.warn` and `logging.warning`. + skip_file_prefixes: Indicates frames to ignore after counting stack levels; similar + to `warnings.warn` *TODO check if this is actually the same effect re:stacklevel*. + Forces stacklevel to max(2, stacklevel). + Default is to exclude anything within `masque`. + low_file_prefixes: Indicates frames to ignore on the other (entry-point) end of the stack, + based on prefixes on their filenames. + low_file_suffixes: Indicates frames to ignore on the other (entry-point) end of the stack, + based on suffixes on their filenames. + + Returns: + Formatted trimmed stack trace + """ + if skip_file_prefixes: + stacklevel = max(2, stacklevel) + + stack = traceback.extract_stack() + + bad_inds = [ii + 1 for ii, frame in enumerate(stack) + if frame.filename.startswith(low_file_prefixes) or frame.filename.endswith(low_file_suffixes)] + first_ok = max([0] + bad_inds) + + last_ok = -stacklevel - 1 + while last_ok >= -len(stack) and stack[last_ok].filename.startswith(skip_file_prefixes): + last_ok -= 1 + + if selected := stack[first_ok:last_ok + 1]: + pass + elif selected := stack[:-stacklevel]: + pass + else: + selected = stack + return ''.join(traceback.format_list(selected)) diff --git a/masque/ports.py b/masque/ports.py index f12706e..a8aaa0c 100644 --- a/masque/ports.py +++ b/masque/ports.py @@ -1,7 +1,5 @@ from typing import overload, Self, NoReturn, Any from collections.abc import Iterable, KeysView, ValuesView, Mapping -import warnings -import traceback import logging import functools from collections import Counter @@ -14,7 +12,7 @@ from numpy.typing import ArrayLike, NDArray from .traits import PositionableImpl, Rotatable, PivotableImpl, Copyable, Mirrorable from .utils import rotate_offsets_around -from .error import PortError +from .error import PortError, format_stacktrace logger = logging.getLogger(__name__) @@ -307,9 +305,9 @@ class PortList(metaclass=ABCMeta): msg = 'Ports have conflicting types:\n' for nn, (kk, vv) in enumerate(connections.items()): if type_conflicts[nn]: - msg = ''.join(traceback.format_stack()) + '\n' + msg - warnings.warn(msg, stacklevel=2) msg += f'{kk} | {a_types[nn]}:{b_types[nn]} | {vv}\n' + msg += '\nStack trace:\n' + format_stacktrace() + logger.warning(msg) a_offsets = numpy.array([pp.offset for pp in a_ports]) b_offsets = numpy.array([pp.offset for pp in b_ports]) @@ -522,9 +520,9 @@ class PortList(metaclass=ABCMeta): msg = 'Ports have conflicting types:\n' for nn, (kk, vv) in enumerate(map_in.items()): if type_conflicts[nn]: - msg = ''.join(traceback.format_stack()) + '\n' + msg - warnings.warn(msg, stacklevel=2) msg += f'{kk} | {s_types[nn]}:{o_types[nn]} | {vv}\n' + msg += '\nStack trace:\n' + format_stacktrace() + logger.warning(msg) rotations = numpy.mod(s_rotations - o_rotations - pi, 2 * pi) if not has_rot.any(): From 006e7c428c6c96924936ab714e600e00276eec42 Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Thu, 30 Oct 2025 01:15:20 -0700 Subject: [PATCH 3/4] [ports] make port mismatch deltas more obvious --- masque/ports.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/masque/ports.py b/masque/ports.py index a8aaa0c..b56ad70 100644 --- a/masque/ports.py +++ b/masque/ports.py @@ -544,8 +544,11 @@ class PortList(metaclass=ABCMeta): translations = s_offsets - o_offsets if not numpy.allclose(translations[:1], translations): msg = 'Port translations do not match:\n' + common_translation = numpy.min(translations, axis=0) + msg += f'Common: {common_translation} \n' + msg += 'Deltas:\n' for nn, (kk, vv) in enumerate(map_in.items()): - msg += f'{kk} | {translations[nn]} | {vv}\n' + msg += f'{kk} | {translations[nn] - common_translation} | {vv}\n' raise PortError(msg) return translations[0], rotations[0], o_offsets[0] From 3e88ed94382c8b11f781c87b31743ee972f7b973 Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Thu, 30 Oct 2025 01:15:44 -0700 Subject: [PATCH 4/4] [file.svg] use logger.warning over warnings.warn (for flexibility) --- masque/file/svg.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/masque/file/svg.py b/masque/file/svg.py index 148f6d4..859c074 100644 --- a/masque/file/svg.py +++ b/masque/file/svg.py @@ -2,7 +2,7 @@ SVG file format readers and writers """ from collections.abc import Mapping -import warnings +import logging import numpy from numpy.typing import ArrayLike @@ -12,6 +12,9 @@ from .utils import mangle_name from .. import Pattern +logger = logging.getLogger(__name__) + + def writefile( library: Mapping[str, Pattern], top: str, @@ -50,7 +53,7 @@ def writefile( bounds = pattern.get_bounds(library=library) if bounds is None: bounds_min, bounds_max = numpy.array([[-1, -1], [1, 1]]) - warnings.warn('Pattern had no bounds (empty?); setting arbitrary viewbox', stacklevel=1) + logger.warning('Pattern had no bounds (empty?); setting arbitrary viewbox', stacklevel=1) else: bounds_min, bounds_max = bounds @@ -117,7 +120,7 @@ def writefile_inverted( bounds = pattern.get_bounds(library=library) if bounds is None: bounds_min, bounds_max = numpy.array([[-1, -1], [1, 1]]) - warnings.warn('Pattern had no bounds (empty?); setting arbitrary viewbox', stacklevel=1) + logger.warning('Pattern had no bounds (empty?); setting arbitrary viewbox', stacklevel=1) else: bounds_min, bounds_max = bounds