Enhance Monitor dict, adding primary monitor and name attributes #469
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,73 @@ | ||
| # This is part of the MSS Python's module. | ||
| # Source: https://github.com/BoboTiG/python-mss. | ||
| from __future__ import annotations | ||
|
|
||
| from typing import TYPE_CHECKING, Any, Callable, NamedTuple | ||
|
|
||
| Pixel = tuple[int, int, int] | ||
| Pixels = list[tuple[Pixel, ...]] | ||
|
|
||
|
|
||
| class Monitor: | ||
|
Contributor
There was a problem hiding this comment. So far, I've been careful to not change any existing aspects of the public API surface. I haven't talked with @BoboTiG about that, though, so he may have different priorities. This is an example of something that would be a change to that surface. The usage would still be API-compatible, but the signature wouldn't: something using The alternative would be to have I know I'm the one that suggested using As another example, I've got one change in the works that affects a lot of What do you think, @halldorfannar and @BoboTiG?
Owner
There was a problem hiding this comment. I would prefer to not touch the current MSS-specific public API (primary functions and types). The example about The code being used for years still needs to work without changes after we ship the next release. And for the next major release, I am also kind of against breaking the API since this is not really necessary. How does it sound to both of you?
Contributor
There was a problem hiding this comment. I want to make sure we’re all aligned on terminology and expectations, since this is really about policy going forward. First off: I completely agree with the priority of protecting existing users. MSS has earned a lot of trust, and preserving working code across releases is hugely important. Nothing here is meant to undermine that. What I’m trying to clarify is how we define “public API” and “breaking change,” especially in Python, where the line can be fuzzy. I’m assuming we’re roughly following Semantic Versioning principles. SemVer only really works if we agree on what the public API is. In Python, the usual convention is “no leading underscore == public,” but MSS historically hasn’t been strict about that (such as the One thing I’ve personally been treating as part of the public API is type declarations, not just runtime behavior. That’s increasingly important now that many users run MyPy or Pyright in CI. A type signature is effectively a promise, even if Python itself won’t enforce it at runtime. That leads to an important distinction:
They matter in different ways. Python APIs are famously hard to change without some theoretical breakage. It’s basically impossible to guarantee that no user code could ever be affected — even adding a key to a dict could break someone doing something exotic. So instead of “never break anything,” I’ve been thinking in terms of typical usage. If something would only break highly unusual or contrived code paths, that carries much less weight than breaking common, idiomatic usage. With that framing, here’s how I’ve been thinking about changes:
The Now, tying this back to your response: when you say “Monitor should be kept backward compatible,” I fully agree at runtime. Where I want to be careful is whether we’re also committing to never changing exposed types, even at major versions. The reason this matters is future performance work. For example, today Most OS capture APIs already give us a writable buffer in CPU memory. Python can expose that safely via From a runtime perspective, this change would be almost invisible to most users. From a typing perspective, however, the distinction matters more. MSS would need to widen its type declarations (for example, from That’s the kind of change I’m trying to plan carefully around. I don’t want to sneak it in under a minor release — but I do think it’s reasonable to allow this sort of evolution at a major version boundary, because it enables MSS to better deliver on its “ultra-fast” goal. So my proposed policy, in short, would be:
My intent here isn’t to justify breaking working code, but to make sure we have room to improve performance and structure without surprising users or their tooling. I’m taking this opportunity to agree on the rules now, so future work doesn’t turn into a philosophical debate every time we touch a boundary. Curious if that framing matches how you’re thinking about it, or if you see things differently.
Owner
There was a problem hiding this comment. Your words are perfectly aligned with my vision, I would not have been able to better phrase it :) I tried to follow SemVer from the beginning, hopefully enough that it made sense so far. In my perspective, and I was too lazy at the beginning to properly make those objects private, all objects used by MSS to power MSS content (like About the MSS API already used by thousands of people, I do not close the door to improvements, and sometimes necessary breaking changes. If that deserves MSS, then lets do it! As I am typing, I am thinking that if we change the type of All in all, I am not a conservator, I know pretty well things must change if we want to move forward. Lets doing it with parcimony, and keep users in mind everytime since I would prefer to lower their burden at each release. I am also totally aligned about preventing philosophy talks at each change (I am not a meeting person, I prefer action first :)
Contributor
Author
There was a problem hiding this comment. I enjoy this conversation. It is the right discussion to have, even if it's happening as fallout from my draft MR 😄 I am also a stickler for SemVer. I have followed it on many projects to the letter, and even though it has felt constricting and painful at times, my customers have always thanked me for it. One of the reasons I made this MR a draft was exactly the uneasy feeling I got when I saw the fallout from changing the Monitor type. Even though I was bending over backwards to ensure runtime compatibility there were still issues, as @jholveck has rightly pointed out. I also just don't like it when a supposedly simple change spreads into multiple files. It feels like a smell, something is off. My suggestion therefore is that we keep Monitor as a dict type for now. But we add a TODO in there (or whatever mechanism this project prefers for future annnouncement) about this becoming a Class type in the next major release. That way we announce our intent. I would also like for us to do this with other potential changes we may want to make in the future. If they require changes that break the API we need to target them for the next major release. As part of release planning we should collect those somewhere, so that we can look up the next major release and see what things we are planning to break with that release. I emphasize planning because in my experience we usually find that not all changes get implemented (some are deemed low-value while others have in the light of time and experience been deemed a bad idea 😆). Finally, I think we should get low-hanging fruit (like this Monitor improvement) into shape so it causes no breakage and cut a release soon. We should then discuss when the next major release is planned, so we can start aligning changes with the right release (major or minor).
Owner
There was a problem hiding this comment. A TODO could be added right above the As MSS is quite stable, there is no really version schedule. When there is a known problem with a fix, or an improvement done, then I cut a fresh version. Since the latest available MSS version, changes to be released are huge! Cutting a new release soon would allow us to break things for the next version, which would be a major. So yes, maybe sticking to
Contributor
Author
There was a problem hiding this comment. All righty then! This is a smaller change now which I think is better for everybody involved. I have removed the "Draft" label. Please review at your leisure. |
||
| """Represents a display monitor with its position and dimensions. | ||
|
|
||
| :param left: The x-coordinate of the upper-left corner. | ||
| :param top: The y-coordinate of the upper-left corner. | ||
| :param width: The width of the monitor. | ||
| :param height: The height of the monitor. | ||
| :param is_primary: Whether this is the primary monitor. | ||
| :param name: The device name of the monitor (platform-specific). | ||
| """ | ||
|
|
||
| __slots__ = ("height", "is_primary", "left", "name", "top", "width") | ||
|
|
||
| def __init__( # noqa: PLR0913 | ||
| self, | ||
| left: int, | ||
| top: int, | ||
| width: int, | ||
| height: int, | ||
| *, | ||
| is_primary: bool = False, | ||
| name: str = "", | ||
| ) -> None: | ||
| self.left = left | ||
| self.top = top | ||
| self.width = width | ||
| self.height = height | ||
| self.is_primary = is_primary | ||
| self.name = name | ||
|
|
||
| def __repr__(self) -> str: | ||
| return ( | ||
| f"Monitor(left={self.left}, top={self.top}, width={self.width}, " | ||
| f"height={self.height}, is_primary={self.is_primary}, name={self.name!r})" | ||
| ) | ||
|
|
||
| def __getitem__(self, key: str) -> int | bool: | ||
| """Provide dict-like access for backward compatibility.""" | ||
| try: | ||
| return getattr(self, key) | ||
| except AttributeError as exc: | ||
| raise KeyError(key) from exc | ||
|
|
||
| def __setitem__(self, key: str, value: int | bool) -> None: | ||
| """Provide dict-like setitem for backward compatibility.""" | ||
| if not hasattr(self, key): | ||
| raise KeyError(key) | ||
| setattr(self, key, value) | ||
|
|
||
| def keys(self) -> list[str]: | ||
| """Provide dict-like keys() for backward compatibility.""" | ||
| return list(self.__slots__) | ||
|
|
||
| def __contains__(self, key: str) -> bool: | ||
| """Provide dict-like 'in' operator for backward compatibility.""" | ||
| return hasattr(self, key) and key in self.__slots__ | ||
|
|
||
|
|
||
| Monitors = list[Monitor] | ||
|
|
||
| if TYPE_CHECKING: | ||
| CFunctions = dict[str, tuple[str, list[Any], Any]] | ||
| CFunctionsErrChecked = dict[str, tuple[str, list[Any], Any, Callable | None]] | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.