From ff43bb30592ad30e5fab98034daefce2e69535f1 Mon Sep 17 00:00:00 2001 From: Joel Ray Holveck Date: Mon, 24 Aug 2026 23:28:19 -0700 Subject: [PATCH 1/3] Fix several small things found during PR #566 review While reviewing PR #566, I had some small cleanups. Some of these were in the PR code, but many were things I found in nearby code during the review. Rather than hold up the commit, I put these in this new PR. * Prefer Sphinx-friendly docstrings for attributes on Monitor * Use primary_monitor instead of monitors[1] in multiple places. * Remove :returns: when it's obvious. * Clarify "string-key access" in the docs. * Rename _RandROutputIds -> _RandROutputInfo, since it's not the ID. In fact, it doesn't even include the XID that XRandR uses. (The unique_id is something different, a URL encoding of certain EDID data.) * Fix typo * Remove dict-oriented phrasing in some docstrings for methods that accept or return a Monitor or Region. * Remove "See :meth:`monitors ` for monitor object details." from grab, since the types it accepts now define the required format. * Validate regions passed to grab, and coerce coordinates to ints. * Make the docstring for "MSS.monitors" more user-oriented, instead of developer-oriented. * Handle the case of an incorrect type passed to grab. Note: Should this be a TypeError or a ScreenShotError? * In MSS.save: * Use the local time zone instead of UTC. * Use the same date for all filenames. * Capture all the screenshots before returning. * Return a Sequence instead of a generator Iterator. A Sequence seems to be a more reasonable type. This also removes the need to consume the return value to complete the operation. This is an incompatible change, since next can't be called on a sequence. However, both the old and new versions work with for loops or with next(iter(sct.save(...))). * Where example monitor attributes (not all of them) were listed in a bulleted list, change those to prose text and make it clear that they aren't an exhaustive list. I have two notes from #566 that I didn't address in this PR. Regarding tests/test_models.py: > I question the utility of several of the tests in this file, but I'm not going to say that they need to be deleted. Regarding changes to docs/source/usage.rst: > This "Capturing Screenshots" section is meant to be a "how to get started" introduction to MSS. It tries to focus on the essentials, and avoid confusing new users. > > With that in mind, most of this seems to be just an unnecessary distraction. It increases the cognitive load on new users. For instance, are new users who are just learning the basics likely to care that **monitor unpacking is not supported? It's just extra stuff that they don't have to care about, but still have to spend cognitive cycles to decide if they care. > > For the purposes of this flow, I think that explaining all about a Monitor object may be a bit too much. I might suggest putting the details in a separate section or subsection, or even just using some cross-references into the API reference for some details. Putting some of this into a separate section can help with cognitive chunking, and let the user get a basic flow before trying to absorb a lot of the details. --- demos/cat-detector.py | 5 +- demos/tinytv-stream.py | 3 +- docs/source/examples/custom_cls_image.py | 2 +- docs/source/release-history/v10.2.0.md | 15 +--- docs/source/release-history/v11.0.0.md | 16 +++- docs/source/usage.rst | 2 +- src/mss/base.py | 103 +++++++++++++---------- src/mss/linux/base.py | 32 +++---- src/mss/models.py | 52 +++++++++--- src/tests/test_implementation.py | 2 +- src/tests/test_save.py | 4 +- 11 files changed, 138 insertions(+), 98 deletions(-) diff --git a/demos/cat-detector.py b/demos/cat-detector.py index e471f407..67c3f10f 100755 --- a/demos/cat-detector.py +++ b/demos/cat-detector.py @@ -109,9 +109,6 @@ import torchvision.models.detection import torchvision.transforms.v2 -# You'll also need to install MSS and Pillow, such as with "pip install mss pillow". -from PIL import Image - import mss # The model will identify objects even if they only vaguely look like something. It also tell us a score of how @@ -221,7 +218,7 @@ def main() -> None: cat_label = model_labels.index("cat") with mss.MSS() as sct: - monitor = sct.monitors[1] + monitor = sct.primary_monitor # Compute the minimum size, in square pixels, that we'll consider reliable. img_area = monitor.width * monitor.height diff --git a/demos/tinytv-stream.py b/demos/tinytv-stream.py index 86d5c6f7..acf471ad 100755 --- a/demos/tinytv-stream.py +++ b/demos/tinytv-stream.py @@ -142,6 +142,8 @@ from collections import deque from typing import TYPE_CHECKING, Literal +# You'll want to install the following packages, through pip or the like: +# mss pyserial pillow prettytable import serial from PIL import Image, ImageOps from prettytable import PrettyTable, TableStyle @@ -517,7 +519,6 @@ def _capture_area_type(value: str) -> Region: integers. :param value: The capture area string to validate. - :returns: Capture region. :raises argparse.ArgumentTypeError: If the format is invalid or extents are non-positive. """ diff --git a/docs/source/examples/custom_cls_image.py b/docs/source/examples/custom_cls_image.py index 3226e598..07001426 100644 --- a/docs/source/examples/custom_cls_image.py +++ b/docs/source/examples/custom_cls_image.py @@ -24,5 +24,5 @@ def __init__(self, data: bytearray, region: Region, **_: Any) -> None: with mss.MSS() as sct: sct.cls_image = SimpleScreenShot - image = sct.grab(sct.monitors[1]) + image = sct.grab(sct.primary_monitor) # ... diff --git a/docs/source/release-history/v10.2.0.md b/docs/source/release-history/v10.2.0.md index 5794fab1..e053254c 100644 --- a/docs/source/release-history/v10.2.0.md +++ b/docs/source/release-history/v10.2.0.md @@ -260,22 +260,11 @@ In 11.0, monitor dictionaries will become a dedicated **`Monitor` class**. To maintain compatibility: -- string-key access will temporarily continue to work - -```python -monitor["left"] -monitor["top"] -``` - +- string-key access (dictionary-style) will temporarily continue to work: `monitor["left"]`, `monitor["top"]`, etc. - `grab()` will continue accepting dictionaries The compatibility access does not make `Monitor` a complete mapping. Migrate dictionary methods, membership tests, and -unpacking to attribute access: - -```python -monitor.left -monitor.top -``` +unpacking to attribute access: `monitor.left`, `monitor.top`, etc. If you use type annotations, you can switch to the provided `Monitor` type: diff --git a/docs/source/release-history/v11.0.0.md b/docs/source/release-history/v11.0.0.md index 4cd50330..b45792cb 100644 --- a/docs/source/release-history/v11.0.0.md +++ b/docs/source/release-history/v11.0.0.md @@ -31,7 +31,7 @@ application code from accidentally changing the geometry or metadata held by an {py:class}`mss.MSS` instance. Use attributes to read geometry and metadata: ```python -monitor = sct.monitors[1] +monitor = sct.primary_monitor # or sct.monitors[1], etc. print(monitor.left, monitor.top, monitor.width, monitor.height) print(monitor.is_primary, monitor.name, monitor.unique_id, monitor.output) region = monitor.as_region() @@ -84,10 +84,22 @@ See the documentation section {ref}`accessing_pixel_data` for details. ### General Improvements -The MSS context object will now always surface inner exceptions, even if `__exit__` may also generate an exception during tear-down. +The MSS context object will now always surface inner exceptions, even if `__exit__` may also generate an exception +during tear-down. + +The {py:meth}`mss.MSS.save` method now returns a {py:class}`Sequence` of strings, rather than an {py:class}`Iterator` of +strings. Code that used `next(sct.save(...))` should now use `sct.save(...)[0]` (easy to write) or +`next(iter(sct.save(...)))` (compatible with both old and new versions of MSS). Code using for loops or other +constructs based on {py:class}`Iterable` (rather than {py:class}`Iterator`) will continue to work as-is. + +If {py:meth}`mss.MSS.save` is given a template with `{date}` in it, the dates are now in the local time zone, instead of +UTC. ## Command-line changes +If the `--output` / `-o` flag is used with `{date}` fields, dates in the filename are now in the local time zone, +instead of UTC. The old behavior can be restored by setting the environment variable `TZ=UTC`. + When invoked from the command line, the `--coordinates` flag can specify coordinates in the format traditionally used by X11: WIDTHxHEIGHT+LEFT+TOP. Additionally, whether using comma-style or X-style coordinates, a negative left or top can be used to specify insets from the right or bottom edge of the specified monitor. diff --git a/docs/source/usage.rst b/docs/source/usage.rst index 68938e59..04af339a 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -56,7 +56,7 @@ Each entry is an immutable :py:class:`mss.models.Monitor`. Its geometry is avai ``width``, and ``height`` attributes. The ``is_primary``, ``name``, ``unique_id``, and ``output`` metadata attributes are ``None`` when unavailable:: - monitor = sct.monitors[1] + monitor = sct.primary_monitor print(monitor.width, monitor.height) Call ``monitor.as_region()`` when you need its geometry as a :py:class:`mss.models.Region`:: diff --git a/src/mss/base.py b/src/mss/base.py index 24557365..3529de32 100644 --- a/src/mss/base.py +++ b/src/mss/base.py @@ -6,7 +6,6 @@ import platform import warnings from abc import ABC, abstractmethod -from copy import copy from datetime import datetime from threading import Lock from typing import TYPE_CHECKING, Any @@ -17,7 +16,7 @@ from mss.tools import to_png if TYPE_CHECKING: - from collections.abc import Callable, Iterator + from collections.abc import Callable, Sequence from types import TracebackType from typing_extensions import Buffer, Self @@ -299,31 +298,44 @@ def close(self) -> None: def grab(self, region: Monitor | Region | dict[str, Any] | tuple[int, int, int, int], /) -> ScreenShot: """Retrieve screen pixels for a given region. - ``region`` can be a :class:`mss.models.Monitor`, a :class:`mss.models.Region`, a region dictionary, or a tuple - like the one :py:func:`PIL.ImageGrab.grab` accepts: ``(left, top, right, bottom)``. + ``region`` can be a :class:`mss.models.Monitor`, a + :class:`mss.models.Region`, a region dictionary, or a tuple + like the one :py:func:`PIL.ImageGrab.grab` accepts: ``(left, + top, right, bottom)``. :param region: The coordinates and size of the box to capture. - See :meth:`monitors ` for monitor object details. :returns: Screenshot of the requested region. """ if isinstance(region, tuple): + if len(region) != 4: # noqa: PLR2004 + msg = "Tuples for grab must have exactly four elements: left, top, right, bottom" + raise ScreenShotError(msg) grab_region = Region( - left=region[0], - top=region[1], - width=region[2] - region[0], - height=region[3] - region[1], + left=int(region[0]), + top=int(region[1]), + width=int(region[2] - region[0]), + height=int(region[3] - region[1]), ) elif isinstance(region, Monitor): grab_region = region.as_region() elif isinstance(region, Region): - grab_region = copy(region) + # Make a copy, in case the user changes the Region object later. Also, coerce the elements to ints. + grab_region = Region( + left=int(region.left), + top=int(region.top), + width=int(region.width), + height=int(region.height), + ) elif isinstance(region, dict): grab_region = Region( - left=region["left"], - top=region["top"], - width=region["width"], - height=region["height"], + left=int(region["left"]), + top=int(region["top"]), + width=int(region["width"]), + height=int(region["height"]), ) + else: + msg = f"Capture area must be a Region, Monitor, tuple, or dict: {region!r}" + raise ScreenShotError(msg) if grab_region.width <= 0 or grab_region.height <= 0: msg = f"Region has zero or negative size: {grab_region!r}" @@ -344,25 +356,21 @@ def grab(self, region: Monitor | Region | dict[str, Any] | tuple[int, int, int, @property def monitors(self) -> Monitors: """Get positions of all monitors. - If the monitor has rotation, you have to deal with it - inside this method. - This method has to fill ``self._monitors`` with all information - and use it as a cache: + If a monitor is rotated, its dimensions reflect its displayed + orientation. In other words, height is the visible height, not + the number of scanout lines. - - ``self._monitors[0]`` is all monitors together - - ``self._monitors[N]`` is monitor N (with N > 0) + The first element, ``monitors[0]``, is all the monitors + together. It is the virtual desktop, holding all the monitors. + The remaining elements, ``monitors[N]`` (with N > 0), are the + individual displays. - Each :class:`mss.models.Monitor` has: + If monitors are being mirrored, the list will only include one + of the mirrored copies. - - ``left``: the x-coordinate of the upper-left corner - - ``top``: the y-coordinate of the upper-left corner - - ``width``: the width - - ``height``: the height - - ``is_primary``: true or false when known, otherwise ``None`` - - ``name``: human-readable device name, or ``None`` - - ``unique_id``: platform-specific stable identifier, or ``None`` - - ``output``: Linux output name compatible with xrandr, or ``None`` + .. seealso:: + :py:attr:`primary_monitor` """ with self._lock: if self._monitors is None: @@ -374,9 +382,9 @@ def monitors(self) -> Monitors: def primary_monitor(self) -> Monitor: """Get the primary monitor. - Returns the monitor marked as primary. If no monitor is marked as primary - (or the platform doesn't support primary monitor detection), returns the - first monitor (at index 1). + Returns the monitor marked as primary. If no monitor is marked + as primary (or the platform doesn't support primary monitor + detection), returns the first monitor (at index 1). :raises ScreenShotError: If no monitors are available. @@ -403,7 +411,7 @@ def save( mon: int = 0, output: str = "monitor-{mon}.png", callback: Callable[[str], None] | None = None, - ) -> Iterator[str]: + ) -> Sequence[str]: """Grab a screenshot and save it to a file. :param int mon: The monitor to screenshot (default=0). ``-1`` @@ -416,19 +424,26 @@ def save( unavailable. :param typing.Callable callback: Called before saving the screenshot; receives the ``output`` argument. - :return: Created file(s). + :return: Names of created file(s). + + .. version-changed:: 11.0.0 + Prior to this version an Iterator was returned, rather than + a Sequence. """ monitors = self.monitors if not monitors: msg = "No monitor found." raise ScreenShotError(msg) + rv: list[str] = [] + + date = datetime.now().astimezone() if mon == 0: # One screenshot by monitor for idx, monitor in enumerate(monitors[1:], 1): fname = output.format( mon=idx, - date=datetime.now(UTC) if "{date" in output else None, + date=date, top=monitor.top, left=monitor.left, width=monitor.width, @@ -442,7 +457,8 @@ def save( callback(fname) sct = self.grab(monitor) to_png(sct.rgb, sct.size, level=self.compression_level, output=fname) - yield fname + rv.append(fname) + else: # A screenshot of all monitors together or # a screenshot of the monitor N. @@ -452,10 +468,9 @@ def save( except IndexError as exc: msg = f"Monitor {mon!r} does not exist." raise ScreenShotError(msg) from exc - - output = output.format( + fname = output.format( mon=mon, - date=datetime.now(UTC) if "{date" in output else None, + date=date, top=monitor.top, left=monitor.left, width=monitor.width, @@ -466,17 +481,19 @@ def save( output=monitor.output, ) if callable(callback): - callback(output) + callback(fname) sct = self.grab(monitor) - to_png(sct.rgb, sct.size, level=self.compression_level, output=output) - yield output + to_png(sct.rgb, sct.size, level=self.compression_level, output=fname) + rv.append(fname) + + return rv def shot(self, /, **kwargs: Any) -> str: """Helper to save the screenshot of the 1st monitor, by default. You can pass the same arguments as for :meth:`save`. """ kwargs["mon"] = kwargs.get("mon", 1) - return next(self.save(**kwargs)) + return self.save(**kwargs)[0] @staticmethod def _merge(screenshot: ScreenShot, cursor: ScreenShot, /) -> ScreenShot: diff --git a/src/mss/linux/base.py b/src/mss/linux/base.py index e55e6bdf..39098f9e 100644 --- a/src/mss/linux/base.py +++ b/src/mss/linux/base.py @@ -26,7 +26,7 @@ ALL_PLANES = 0xFFFFFFFF # XCB doesn't define AllPlanes -class _RandROutputIds(TypedDict, total=False): +class _RandROutputInfo(TypedDict, total=False): name: str unique_id: str output: str @@ -233,13 +233,13 @@ def _randr_get_edid_atom(self) -> xcb.Atom | None: # Formerly, "EDID" was known as "EdidData". I don't know when it changed. return xcb.intern_atom(self.conn, "EdidData", only_if_exists=True) - def _randr_output_ids( + def _randr_output_info( self, output: xcb.RandrOutput, timestamp: xcb.Timestamp, edid_atom: xcb.Atom | None, /, - ) -> _RandROutputIds: + ) -> _RandROutputInfo: if self.conn is None: msg = "Cannot identify monitors while the connection is closed" raise ScreenShotError(msg) @@ -249,7 +249,7 @@ def _randr_output_ids( msg = "Display configuration changed while detecting monitors." raise ScreenShotError(msg) - rv: _RandROutputIds = {} + rv: _RandROutputInfo = {} output_name_arr = xcb.randr_get_output_info_name(output_info) rv["output"] = bytes(output_name_arr).decode("utf_8", errors="replace") @@ -312,11 +312,11 @@ def _monitors_from_randr_monitors( monitors_reply = xcb.randr_get_monitors(self.conn, self.drawable, 1) timestamp = monitors_reply.timestamp for randr_monitor in xcb.randr_get_monitors_monitors(monitors_reply): - output_ids: _RandROutputIds = {} + output_info: _RandROutputInfo = {} if randr_monitor.nOutput > 0: outputs = xcb.randr_monitor_info_outputs(randr_monitor) chosen_output = self._choose_randr_output(outputs, primary_output) - output_ids = self._randr_output_ids(chosen_output, timestamp, edid_atom) + output_info = self._randr_output_info(chosen_output, timestamp, edid_atom) monitors.append( Monitor( @@ -324,15 +324,12 @@ def _monitors_from_randr_monitors( top=randr_monitor.y, width=randr_monitor.width, height=randr_monitor.height, - # Under XRandR, it's legal for no monitor to be primary. In - # this case, case MSSBase.primary_monitor will return the - # first monitor. That said, we note in the Monitor that we - # explicitly are told by XRandR that all of the monitors are - # not primary. (This is distinct from the XRandR 1.2 path, - # which doesn't have any information about primary - # monitors.) + # Under XRandR, it's legal for no monitor to be primary. In this case, MSSBase.primary_monitor will + # return the first monitor. That said, we note in the Monitor that we explicitly are told by XRandR + # that all of the monitors are not primary. (This is distinct from the XRandR 1.2 path, which + # doesn't have any information about primary monitors.) is_primary=bool(randr_monitor.primary), - **output_ids, + **output_info, ), ) @@ -366,7 +363,7 @@ def _monitors_from_randr_crtcs( continue outputs = xcb.randr_get_crtc_info_outputs(crtc_info) chosen_output = self._choose_randr_output(outputs, primary_output) - output_ids = self._randr_output_ids(chosen_output, timestamp, edid_atom) + output_info = self._randr_output_info(chosen_output, timestamp, edid_atom) # The concept of primary outputs was added in XRandR 1.3. We distinguish between "all the monitors are # not primary" (RRGetOutputPrimary returned XCB_NONE, a valid case) and "we have no way to get # information about the primary monitor": in the latter case, is_primary is None. @@ -377,7 +374,7 @@ def _monitors_from_randr_crtcs( width=crtc_info.width, height=crtc_info.height, is_primary=chosen_output == primary_output if primary_output is not None else None, - **output_ids, + **output_info, ), ) @@ -441,8 +438,7 @@ def _grab_xgetimage(self, region: Region, /) -> bytearray: Used by the XGetImage backend and by the XShmGetImage backend in fallback mode. - :param region: Rectangle specifying ``left``, ``top``, - ``width``, and ``height`` to capture. + :param region: Desktop area to capture. :returns: A screenshot object containing the captured region. """ diff --git a/src/mss/models.py b/src/mss/models.py index 195060c3..553a8d30 100644 --- a/src/mss/models.py +++ b/src/mss/models.py @@ -20,29 +20,57 @@ class Region: class Monitor: """Monitor geometry and optional platform metadata. - The optional metadata attributes are: - - - ``is_primary``: whether this is the primary monitor; ``None`` means - the platform could not determine it. - - ``name``: the human-readable device name; ``None`` means it is - unavailable. - - ``unique_id``: the platform-specific stable identifier; ``None`` - means it is unavailable. - - ``output``: the Linux output name compatible with xrandr; ``None`` - means it is unavailable or does not apply to the platform. + .. seealso:: + - :py:attr:`.MSS.monitors` + - :py:attr:`.MSS.primary_monitor` + + .. version-changed:: 11.0.0 + Prior to this version, ``Monitor`` was an alias for ``dict[str, int]``. + In MSS 11, it is still possible to access attributes with dict-style + string-key access, such as ``monitor["left"]``, but this + behavior is deprecated and will be removed in a later version. """ + #: The monitor's left edge within the entire virtual desktop. left: int + #: The monitor's top edge within the entire virtual desktop. top: int width: int height: int - is_primary: bool | None = None + #: The human-readable name of this monitor, typically the brand + #: and model. + #: + #: .. version-added:: 10.2.0 name: str | None = None + #: Whether this is the primary monitor, according to the operating + #: system. If MSS can't determine the primary monitor, this will + #: be ``None`` for all monitors, although + #: :py:attr:`.MSS.primary_monitor` will still return a monitor + #: (the first one). + #: + #: .. version-added:: 10.2.0 + is_primary: bool | None = None + #: The platform-specific stable identifier. This is + #: generally stable across reboots, or ordinary disconnection / + #: reconnection, but may change when the display hardware or + #: connection topology changes. + #: + #: .. version-added:: 10.2.0 unique_id: str | None = None + #: The short output name, for interfacing with other tools. This + #: is only currently populated by Linux, where it is the name used + #: by xrandr. + #: + #: .. version-added:: 10.2.0 output: str | None = None def as_region(self) -> Region: - """Return this monitor's geometry as a capture region.""" + """Return this monitor's geometry as a capture region. + + .. version-added:: 11.0.0 + Prior to this version, a Region and a Monitor were effectively + the same: a dict with left, top, width, and height entries. + """ return Region(left=self.left, top=self.top, width=self.width, height=self.height) @overload diff --git a/src/tests/test_implementation.py b/src/tests/test_implementation.py index 43d855b6..72738d01 100644 --- a/src/tests/test_implementation.py +++ b/src/tests/test_implementation.py @@ -242,7 +242,7 @@ def test_output_pattern_with_date(self, with_cursor: bool, capsys: pytest.Captur fmt = "sct_{mon}-{date:%Y-%m-%d}.png" for opt in ("-o", "--out"): self._run_main(with_cursor, "-m 1", opt, fmt) - filename = Path(fmt.format(mon=1, date=datetime.now(tz=UTC))) + filename = Path(fmt.format(mon=1, date=datetime.now().astimezone())) captured = capsys.readouterr() assert captured.out.endswith(f"{filename}\n") assert filename.is_file() diff --git a/src/tests/test_save.py b/src/tests/test_save.py index b9b611e8..8457cefe 100644 --- a/src/tests/test_save.py +++ b/src/tests/test_save.py @@ -85,7 +85,7 @@ def capture_filename(value: str) -> None: with mss_impl() as sct: monitor = sct.monitors[1] with pytest.raises(FormattingCompleteError): - next(sct.save(mon=1, output=fmt, callback=capture_filename)) + sct.save(mon=1, output=fmt, callback=capture_filename) assert filename == fmt.format(is_primary=monitor.is_primary, unique_id=monitor.unique_id) @@ -105,5 +105,5 @@ def test_output_format_date_custom(mss_impl: Callable[..., MSS]) -> None: fmt = "sct_{date:%Y-%m-%d}.png" with mss_impl() as sct: filename = sct.shot(mon=1, output=fmt) - assert filename == fmt.format(date=datetime.now(tz=UTC)) + assert filename == fmt.format(date=datetime.now().astimezone()) assert Path(filename).is_file() From 17f3294b683e73e4e32115eca4033adf2c1a0be8 Mon Sep 17 00:00:00 2001 From: Joel Ray Holveck Date: Mon, 24 Aug 2026 23:49:30 -0700 Subject: [PATCH 2/3] Whitespace fix --- src/mss/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mss/models.py b/src/mss/models.py index 553a8d30..7605cde2 100644 --- a/src/mss/models.py +++ b/src/mss/models.py @@ -69,7 +69,7 @@ def as_region(self) -> Region: .. version-added:: 11.0.0 Prior to this version, a Region and a Monitor were effectively - the same: a dict with left, top, width, and height entries. + the same: a dict with left, top, width, and height entries. """ return Region(left=self.left, top=self.top, width=self.width, height=self.height) From 1fd30b590982a92c57e2480d0608c646442fb85c Mon Sep 17 00:00:00 2001 From: Joel Ray Holveck Date: Tue, 25 Aug 2026 19:36:55 -0700 Subject: [PATCH 3/3] Use Region.__post_init__ to coerce args to ints Also, add some more tests to make sure the coercion is working as intended. --- src/mss/base.py | 27 ++++++++++------------- src/mss/models.py | 6 +++++ src/tests/test_implementation.py | 38 +++++++++++++++++++++++++++++--- src/tests/test_models.py | 7 ++++++ 4 files changed, 60 insertions(+), 18 deletions(-) diff --git a/src/mss/base.py b/src/mss/base.py index 3529de32..dd3a175e 100644 --- a/src/mss/base.py +++ b/src/mss/base.py @@ -3,6 +3,7 @@ from __future__ import annotations +import dataclasses import platform import warnings from abc import ABC, abstractmethod @@ -311,27 +312,23 @@ def grab(self, region: Monitor | Region | dict[str, Any] | tuple[int, int, int, msg = "Tuples for grab must have exactly four elements: left, top, right, bottom" raise ScreenShotError(msg) grab_region = Region( - left=int(region[0]), - top=int(region[1]), - width=int(region[2] - region[0]), - height=int(region[3] - region[1]), + left=region[0], + top=region[1], + width=region[2] - region[0], + height=region[3] - region[1], ) elif isinstance(region, Monitor): grab_region = region.as_region() elif isinstance(region, Region): - # Make a copy, in case the user changes the Region object later. Also, coerce the elements to ints. - grab_region = Region( - left=int(region.left), - top=int(region.top), - width=int(region.width), - height=int(region.height), - ) + # Make a copy, in case the user changes the Region object later. We use dataclasses.replace because + # copy.copy doesn't call __post_init__, and we want the type normalization. + grab_region = dataclasses.replace(region) elif isinstance(region, dict): grab_region = Region( - left=int(region["left"]), - top=int(region["top"]), - width=int(region["width"]), - height=int(region["height"]), + left=region["left"], + top=region["top"], + width=region["width"], + height=region["height"], ) else: msg = f"Capture area must be a Region, Monitor, tuple, or dict: {region!r}" diff --git a/src/mss/models.py b/src/mss/models.py index 7605cde2..fa2b20ac 100644 --- a/src/mss/models.py +++ b/src/mss/models.py @@ -15,6 +15,12 @@ class Region: width: int height: int + def __post_init__(self) -> None: + self.left = int(self.left) + self.top = int(self.top) + self.width = int(self.width) + self.height = int(self.height) + @dataclass(frozen=True, slots=True) class Monitor: diff --git a/src/tests/test_implementation.py b/src/tests/test_implementation.py index 72738d01..7dc8851e 100644 --- a/src/tests/test_implementation.py +++ b/src/tests/test_implementation.py @@ -435,7 +435,7 @@ def test_parse_and_normalize_coordinates(geom: str, expected: Region) -> None: assert norm == expected -def test_grab_with_tuple(mss_impl: Callable[..., MSS]) -> None: +def test_grab_with_tuple_and_dict(mss_impl: Callable[..., MSS]) -> None: left = 100 top = 100 right = 500 @@ -444,12 +444,12 @@ def test_grab_with_tuple(mss_impl: Callable[..., MSS]) -> None: height = lower - top # 400px height with mss_impl() as sct: - # PIL like + # PIL style box = (left, top, right, lower) im = sct.grab(box) assert im.size == (width, height) - # MSS like + # MSS pre-11 style box2 = {"left": left, "top": top, "width": width, "height": height} im2 = sct.grab(box2) assert im.size == im2.size @@ -473,6 +473,38 @@ def test_grab_with_region(mss_impl: Callable[..., MSS]) -> None: assert image.rgb == expected.rgb +def test_grab_coerces_floats_to_int(mss_impl: Callable[..., MSS], monkeypatch: pytest.MonkeyPatch) -> None: + left = 100.0 + top = 100.0 + width = 400.0 + height = 400.0 + right = left + width + lower = top + height + expected_region = Region(left=100, top=100, width=400, height=400) + + with mss_impl() as sct: + # The implementation classes use __slots__, so "grab" must be patched on the class. Mock + # isn't a descriptor, so wrap the already-bound method rather than the unbound one, or the + # instance's call (which supplies no "self") won't match the wrapped callable's signature. + mock_grab = Mock(wraps=sct._impl.grab) + monkeypatch.setattr(type(sct._impl), "grab", mock_grab) + + sct.grab((left, top, right, lower)) # type: ignore[arg-type] + mock_grab.assert_called_once_with(expected_region) + mock_grab.reset_mock() + + sct.grab({"left": left, "top": top, "width": width, "height": height}) + mock_grab.assert_called_once_with(expected_region) + mock_grab.reset_mock() + + sct.grab(Region(left=left, top=top, width=width, height=height)) # type: ignore[arg-type] + mock_grab.assert_called_once_with(expected_region) + mock_grab.reset_mock() + + sct.grab(Monitor(left=left, top=top, width=width, height=height)) # type: ignore[arg-type] + mock_grab.assert_called_once_with(expected_region) + + def test_grab_with_invalid_tuple(mss_impl: Callable[..., MSS]) -> None: with mss_impl() as sct: # Remember that rect tuples are PIL-style: (left, top, right, bottom) diff --git a/src/tests/test_models.py b/src/tests/test_models.py index cd7b68d1..9cdd5a41 100644 --- a/src/tests/test_models.py +++ b/src/tests/test_models.py @@ -23,6 +23,13 @@ def test_region() -> None: assert (region.left, region.top, region.width, region.height) == (5, 6, 7, 8) +def test_region_coerces_fields_to_int() -> None: + region = Region(left=1.1, top=2.2, width=3.9, height=4.5) # type: ignore[arg-type] + + assert (region.left, region.top, region.width, region.height) == (1, 2, 3, 4) + assert all(isinstance(value, int) for value in (region.left, region.top, region.width, region.height)) + + def test_monitor() -> None: monitor = Monitor( left=1,