Add immutable Monitor model by halldorfannar · Pull Request #566 · BoboTiG/python-mss

halldorfannar · 2026-08-03T12:29:39Z

Changes proposed in this PR

Fixes #470

Replace heterogeneous monitor dictionaries with a frozen, slotted Monitor dataclass that exposes typed geometry and
platform metadata attributes.

Update the macOS, GNU/Linux, and Windows backends to produce and consume Monitor objects.
Keep temporary string-key access for migration, while continuing to accept dictionaries and PIL-style tuples in
grab().
Introduce new Region class and as_region factory function for Monitors; refactor code to use these
Preserve immutable monitor descriptions in the video demos by constructing separate ojbects for cropped capture
regions.
Update filename formatting, examples, documentation, release notes, typing, and tests for the new model.
Update AGENTS.md with instructions on how to update test_sdist when files are added, removed, or renamed.

This gives callers a stable, read-only monitor representation without preventing existing applications from passing
dictionary-based capture regions.

Tests added/updated
Documentation updated
Changelog entry added
./check.sh passed

Validation: uv run ./check.sh. The full pytest suite was not run during the final review.

AI assistance disclosure

AI assistance was used to generate this contribution.

Codex assisted with reviewing the implementation, identifying and implementing follow-up fixes in the demos and
ScreenShot initialization, updating the immutability test for static typing, running validation, and drafting this PR
description. The contributor reviewed and directed the API and capture-region decisions.

Replace monitor dictionaries with a frozen, slotted Monitor dataclass while preserving temporary string-key access. Update platform enumeration, legacy grab inputs, filename formatting, typing, tests, examples, and migration documentation.

Some samples would not work correctly. Also prepared for eventually phasing out string like access.

jholveck

I'm not really comfortable with this idea of making synthetic Monitor objects to send down through _impl.grab, rather than sending capture regions as dicts or tuples. Can you elaborate on why you made that choice?

- MSS.grab() still accepts Monitor, dictionaries, and tuples, but normalizes them to CaptureRegion. - Platform backends and custom screenshot classes now receive regions, never synthetic monitors. - Added Monitor.as_capture_region(). - Applied the Linux **output_ids and comment cleanups. - Fixed the demo import grouping. - Documented why monitors are immutable. - Added test_wheel guidance to AGENTS.md.

halldorfannar · 2026-08-04T09:17:11Z

One overall thing. I'm wondering if the name CaptureRegion is too limiting. Maybe Region is better and more futureproof. It doesn't matter what the region will be used for inside MSS. It will always have the same coordinates associated with it. Using it directly for capture is just one application I can think of.

jholveck · 2026-08-04T09:26:48Z

One overall thing. I'm wondering if the name CaptureRegion is too limiting. Maybe Region is better and more futureproof. It doesn't matter what the region will be used for inside MSS. It will always have the same coordinates associated with it. Using it directly for capture is just one application I can think of.

Yeah, I think that Region may be better. For instance, we might use it in a Window object to give the window geometry to the user.

(Besides, as_capture_region felt a bit verbose; it was just the first thing that occurred to me when I wrote that comment.)

Food for thought: should a Region carry with it the metric space (nominal vs. physical)? Probably not practically, and definitely not in this PR, but it may be at least worth passing through the brain once.

halldorfannar · 2026-08-06T19:06:57Z

This is ready to merge.

- Change code to prefer the new attribute access for Monitor and Region. - Update docs - Update tests

- MSS.grab() now snapshots caller-owned Region objects before validation/backend use in src/mss/base.py:318. - Added a custom-image snapshot regression test and separated Region grab coverage. - Migrated five documentation examples to Region, retaining one explicit dictionary compatibility example.

BoboTiG · 2026-08-10T12:43:56Z

/cc @jholveck I think you synced with @halldorfannar already, just checking :)

jholveck

I think it's good as-is. Unlike the previous revision, I don't have any major structural issues here.

I would suggest we commit it as it stands, but I do have several suggestions. Many of these are documentation suggestions. Many others are for code that's just near the edits, and not really part of @halldorfannar 's proposed changes, but I just happened to notice them. I've put in suggestions for most of them, but most of these haven't been tested, or even run them through check.sh.

As I said, I suggest we commit this as it stands, but also consider these other suggestions for follow-up commits, or to split into new issues.

jholveck · 2026-08-12T23:06:13Z

@@ -224,7 +224,7 @@ def main() -> None:
        monitor = sct.monitors[1]


Now that we have primary_monitor, we should use that.

Suggested change

monitor = sct.monitors[1]

monitor = sct.primary_monitor

jholveck · 2026-08-12T23:11:53Z


    :param value: The capture area string to validate.
-    :returns: Dict with 'left', 'top', 'width', 'height' keys.
+    :returns: Capture region.


I normally don't leave in :returns: if it's obvious from the context and return type. I do for dicts, to clarify the keys, but I don't think it's needed for Region.

Suggested change

:returns: Capture region.

jholveck · 2026-08-12T23:15:49Z


 with mss.MSS() as sct:
    sct.cls_image = SimpleScreenShot
    image = sct.grab(sct.monitors[1])


While we're in here...

Suggested change

image = sct.grab(sct.primary_monitor)

jholveck · 2026-08-12T23:17:42Z

 To maintain compatibility:

- dictionary-style access will continue to work
+- string-key access will temporarily continue to work


Just to make it clear even to new programmers:

Suggested change

- string-key access will temporarily continue to work

- string-key (dictionary-style) access will temporarily continue to work

jholveck · 2026-08-12T23:20:12Z

+{py:class}`mss.MSS` instance. Use attributes to read geometry and metadata:
+
+```python
+monitor = sct.monitors[1]


Just to get people in the habit of thinking about primary_monitor instead of monitors[1], I suggest we use that.

Suggested change

monitor = sct.monitors[1]

monitor = sct.primary_monitor # or monitors[1], etc.

jholveck · 2026-08-13T02:22:29Z

+    with mss_impl() as sct:
+        monitor = sct.monitors[1]
+        with pytest.raises(FormattingCompleteError):
+            next(sct.save(mon=1, output=fmt, callback=capture_filename))


Seeing this next makes me wonder: is there a reason that we make save a generator, instead of saving all the screenshots immediately and returning a list? I looked at the commit history, and this actually goes back to the very first MSS code, in 237a744. But it does seem a bit surprising to me that we're doing it this way.

If we want to change it, I can open a new issue.

Looking at that commit makes me nostalgic haha.

I do not remember why I did a generator here, maybe was I thinking about memory resources optimization.

Anyway, we can change it, yes!

jholveck · 2026-08-13T02:29:08Z

+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
+```
+


I might suggest that we either list all the attributes, or even just the four coordinate ones, or if we want a brief list, just put them inline. I think that just a couple inline would be fine here.

Suggested change

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

```

The compatibility access does not make `Monitor` a complete mapping. Migrate dictionary methods, membership tests, and

unpacking to attribute access, such as ``monitor.left`` and ``monitor.top``.

jholveck · 2026-08-13T02:30:23Z

+``width``, and ``height`` attributes.  The ``is_primary``, ``name``, ``unique_id``, and ``output`` metadata attributes
+are ``None`` when unavailable::
+
+    monitor = sct.monitors[1]


Suggested change

monitor = sct.monitors[1]

monitor = sct.primary_monitor

jholveck · 2026-08-13T02:38:54Z

+    monitor = sct.monitors[1]
+    print(monitor.width, monitor.height)
+
+Call ``monitor.as_region()`` when you need its geometry as a :py:class:`mss.models.Region`::


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.

jholveck · 2026-08-13T02:45:14Z

+        elif isinstance(region, dict):
+            grab_region = Region(
+                left=region["left"],
+                top=region["top"],
+                width=region["width"],
+                height=region["height"],
+            )


May want to do type coercion here.

Suggested change

elif isinstance(region, dict):

grab_region = Region(

left=region["left"],

top=region["top"],

width=region["width"],

height=region["height"],

)

elif isinstance(region, dict):

grab_region = Region(

left=int(region["left"]),

top=int(region["top"]),

width=int(region["width"]),

height=int(region["height"]),

)

BoboTiG · 2026-08-13T07:20:04Z

would suggest we commit it as it stands, but I do have several suggestions. Many of these are documentation suggestions. Many others are for code that's just near the edits, and not really part of @halldorfannar 's proposed changes, but I just happened to notice them. I've put in suggestions for most of them, but most of these haven't been tested, or even run them through check.sh.

Agreed. I merge, feel free to open a follow up PR.

BoboTiG · 2026-08-13T07:20:49Z

Thank you @halldorfannar 🥂

halldorfannar added 4 commits August 3, 2026 13:51

feat: add immutable Monitor model

0aae34d

Replace monitor dictionaries with a frozen, slotted Monitor dataclass while preserving temporary string-key access. Update platform enumeration, legacy grab inputs, filename formatting, typing, tests, examples, and migration documentation.

Cleanup and fixes

5cedb7b

Some samples would not work correctly. Also prepared for eventually phasing out string like access.

fix: include monitor model test in sdist manifest

95b92c0

docs: document sdist manifest updates

8042f02

halldorfannar changed the title ~~feat: add immutable Monitor model~~ Add immutable Monitor model Aug 3, 2026

ci: rerun checks

0f1b933

jholveck reviewed Aug 4, 2026

View reviewed changes

halldorfannar added 2 commits August 5, 2026 15:49

Improve wording based on feedback

8268271

Rename CaptureRegion to Region and as_capture_region to as_region

41597a3

halldorfannar marked this pull request as ready for review August 5, 2026 13:53

BoboTiG reviewed Aug 6, 2026

View reviewed changes

Comment thread src/mss/screenshot.py Outdated

halldorfannar added 2 commits August 7, 2026 09:17

Change Region to an attribute class

1475cc3

- Change code to prefer the new attribute access for Monitor and Region. - Update docs - Update tests

BoboTiG approved these changes Aug 8, 2026

View reviewed changes

jholveck approved these changes Aug 13, 2026

View reviewed changes

jholveck mentioned this pull request Aug 13, 2026

Fix X-style geometry parsing #569

Draft

5 tasks

BoboTiG merged commit 5bfbe60 into BoboTiG:main Aug 13, 2026
18 checks passed

		@@ -224,7 +224,7 @@ def main() -> None:
		monitor = sct.monitors[1]

	- string-key access will temporarily continue to work
	- string-key (dictionary-style) access will temporarily continue to work

	monitor = sct.monitors[1]
	monitor = sct.primary_monitor # or monitors[1], etc.

Uh oh!

Conversation

halldorfannar commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this PR

AI assistance disclosure

Uh oh!

jholveck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

halldorfannar commented Aug 4, 2026

Uh oh!

jholveck commented Aug 4, 2026

Uh oh!

halldorfannar commented Aug 6, 2026

Uh oh!

Uh oh!

BoboTiG commented Aug 10, 2026

Uh oh!

jholveck left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BoboTiG commented Aug 13, 2026

Uh oh!

Uh oh!

BoboTiG commented Aug 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

halldorfannar commented Aug 3, 2026 •

edited

Loading