Conversation
See also BoboTiG/python-mss issue BoboTiG#486. The user will always work with a single class: mss.MSS. Differences in implementation, such as platform or capture strategy, are hidden in an internal implementation object held by the mss.MSS object. This allows us to change the implementation, with arbitrary class hierarchies, without worrying about preserving compatibility with internal class names. This deprecates the existing `mss` factory function, although it can easily be kept for as long as needed to give users time to adapt. It also deprecates the existing `mss.{platform}.MSS` types. These are exposed to the user, so somebody calling `mss.{platform}.MSS()` in 10.x can still reasonably expect to get a `mss.{platform}.MSS` object back. However, in 11.0, we can remove the type entirely, and either remove those symbols, or make them deprecated aliases for `mss.MSS`. Where possible, deprecated functionality emits a `DeprecationWarning`. However, note that these are ignored by default, unless triggered by code in `__main__`. Many of the API docs are removed, since this change removes much of the API surface. However, they are still in available for backwards-compatibility. This change adds tests for everything that was in the 10.1 docs, examples, etc, at least at a basic level: for instance, it tests that `mss.linux.MSS` still works as both a constructor and a type (for `isinstance`), and that `mss.linux.ZPIXMAP` still exists (it was listed in the 10.1 docs). The existing code, tests, and docs are changed to use `mss.MSS`.
The mss_impl fixture would add an implicit display= argument, regardless of platform. The code at that time would ignore it, but we should be (and in the previous commit, were) more strict. Change mss_impl to only use display= if appropriate, so we can be more strict in the future. In 10.1, these were allowed at all times, and ignored if the platform didn't use them. Emulate this behavior in mss.MSS (and mss.mss), with DeprecationWarnings, and test.
I'm pretty sure it's unnecessary there. Not sure why it was being done.
|
For context, here is the text of the issue referenced (I don't know if Copilot can see issues in the upstream repo). We agreed on path 2, using a strategy pattern. Executive summaryThis issue’s goal is to support multiple backends while keeping the public MSS API stable for users. The current Linux implementation changed In the future, other platforms will likely need similar backend selection. For example:
Because backend selection happens at initialization time, it does not fit well with the current design where each platform exposes a single concrete class. I propose moving to a strategy-style design:
Benefits of this approach:
Existing user code should continue to work during the transition period. The rest of this issue describes the problem in more detail and discusses possible implementation paths. DetailsI think I should change the way that I currently provide When I added the XCB backend alongside the Xlib backend, I changed it to be a factory function: the choice of the concrete class to use depends on the Longer-term, we don’t just have Linux to consider. The other platforms are likely to need parallel backends too:
These backends are typically going to be largely independent of one another, and inheritance becomes awkward. For instance, the XCB and Xlib code don’t share anything. It doesn’t seem reasonable to put all of a platform’s backends under a common class. This is particularly important because the selection of backend is done at initialization time. As an example, in the Linux case, if MSS detects that We don’t provide a generic In the same way, in the future, I see a few possible paths. I currently prefer the strategy pattern approach (Path 2). Path 1: Factory functionsWe remove the For 10.2.0, make For 11.0, turn For 12.0 (whenever that is), remove We’d still need to choose what factory functions we want to commit to, in order to provide API stability. I’d recommend just the This would require some migration for users:
Path 2: Strategy patternThis is what I’m leaning towards, and I’ve been leaning to it more and more as I’ve been considering the problem. (This issue has been on my mind for a few weeks now.) It lets us freeze the API boundary at one, clear place. The strategy pattern is purpose-built for this kind of situation. The user-visible class provides the user-visible methods, but holds a strategy object that does part of the work. We already have a lot of this in place, by and large: The difference is that, in the strategy pattern, the backend is a separate class from the one that the user sees. It’s held by the user-facing class. This follows a principle that many (but not all) modern object-oriented scholars promote: “composition over inheritance”. The public, user-facing class provides the public API. The private, backend class performs the platform-specific work. The public class holds the backend object and calls it when needed. This would probably make the API more Pythonic. It would let the user refer to and instantiate a In this plan, # Today
sct = mss.linux.MSS()
type(sct) is mss.linux.XShmGetImage
# Proposal
sct = mss.MSS()
type(sct) is mss.MSSCompatibility is easy. We’d need to have I particularly like this option for a few reasons:
Path 3: Constructor as factoryI don’t like this one, but it’s a possibility. We can provide a I think that using |
|
As a contextual note, 10.1 (the most recent release) had only a single Linux backend, one based on Xlib. One major change in 10.2 is the idea of us managing different backends, such as some that might be based on XComposite or capture to CUDA or do other things that require very different implementations. Another use case for multiple backends would be the different screen capture APIs that Windows exposes, which may or may not be applicable depending on the Windows version and use case. Originally, I added the new Linux XCB-based backends (one using XShmGetImage and one using XGetImage), and made As I considered this further, I realized that a larger shift may be necessary, to allow us to have a consistent, stable API, while we change the internals. Rather than having the implementations be classes that inherit from the base class, it would make more sense to use a strategy pattern. One important thing about this history lesson: the new Linux backends have not yet been part of a public release. That also includes the |
|
As context, here is a review of the API that was documented in the most recent public release, 10.1. Some of the docs in 10.1 listed things - implementation details - that really should have never been public, but since we're trying to commit to semver, we don't want to break code that uses them. python-mss 10.1 Public API InventoryThis is a pragmatic inventory of what users were likely relying on in
This is not an exhaustive symbol dump. It is a compatibility checklist for Tag legend:
1) Top-level package surface (
|
There was a problem hiding this comment.
Pull request overview
This PR refactors MSS to a strategy-based design where users interact with a single stable public class (mss.MSS), while platform/backend specifics are encapsulated in internal implementation objects.
Changes:
- Introduces
MSS(public facade) +MSSImplementation(internal strategy interface) and migrates platform backends toMSSImpl*implementations. - Adds/updates compatibility shims (deprecated
mss.mss, deprecatedmss.{platform}.MSS) and adds compatibility-focused tests. - Updates docs, demos, benchmarks, and tests to use
mss.MSSdirectly.
Reviewed changes
Copilot reviewed 59 out of 59 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/mss/base.py | Adds the MSS facade + MSSImplementation interface and central backend selection/caching. |
| src/mss/factory.py | Deprecates mss.mss() factory and forwards to mss.MSS. |
| src/mss/linux/init.py | Introduces deprecated mss.linux.MSS constructor and Linux implementation selection (choose_impl). |
| src/mss/linux/base.py | Renames/refactors XCB base backend into an MSSImplementation with grab/monitors/cursor. |
| src/mss/linux/xgetimage.py | Refactors XGetImage backend into an implementation strategy class. |
| src/mss/linux/xshmgetimage.py | Refactors XShmGetImage backend into an implementation strategy class with fallback. |
| src/mss/linux/xlib.py | Refactors legacy Xlib backend into an implementation strategy class and updates cursor handling. |
| src/mss/windows.py | Adds deprecated mss.windows.MSS wrapper and refactors Windows backend into MSSImplWindows. |
| src/mss/darwin.py | Adds deprecated mss.darwin.MSS wrapper and refactors macOS backend into MSSImplDarwin. |
| src/mss/init.py | Exports MSS (and ScreenShot) at top-level and keeps deprecated mss factory. |
| src/mss/main.py | Updates CLI entry point to use MSS(...) directly. |
| src/tests/conftest.py | Updates mss_impl fixture to construct MSS (optionally injecting DISPLAY on Linux). |
| src/tests/test_implementation.py | Migrates core behavior/CLI tests to MSS + new base types. |
| src/tests/test_gnu_linux.py | Updates Linux backend tests to the new strategy structure and _impl access. |
| src/tests/test_windows.py | Updates Windows tests to use mss.MSS and verify Windows impl internals via _impl. |
| src/tests/test_macos.py | Updates macOS tests to use mss.MSS and patch the internal impl. |
| src/tests/test_xcb.py | Updates xgetimage tests to use MSS(backend=...) and validate impl selection. |
| src/tests/test_find_monitors.py | Updates monitor enumeration tests to type against MSS. |
| src/tests/test_get_pixels.py | Updates pixel-grab tests to use MSS and top-level ScreenShot. |
| src/tests/test_save.py | Updates save/shot tests to use MSS. |
| src/tests/test_primary_monitor.py | Updates primary-monitor tests to use mss.MSS. |
| src/tests/test_tools.py | Updates tool tests typing to MSS. |
| src/tests/test_setup.py | Uses sys.executable -m ... for build/twine commands; expands sdist file list. |
| src/tests/test_issue_220.py | Updates Tkinter regression test to use MSS. |
| src/tests/test_leaks.py | Updates leak/regression tests to use MSS. |
| src/tests/test_cls_image.py | Updates custom cls_image test to use MSS. |
| src/tests/third_party/test_pil.py | Updates PIL integration test typing/imports to MSS. |
| src/tests/third_party/test_numpy.py | Updates NumPy integration test typing/imports to MSS. |
| src/tests/test_compat_10_1.py | Adds explicit compatibility tests for 10.1 documented usage patterns and warnings. |
| src/tests/test_compat_exports.py | Adds compatibility tests for top-level export surface and MSSBase alias. |
| src/tests/test_compat_linux_api.py | Adds Linux symbol re-export compatibility tests. |
| src/tests/bench_general.py | Updates benchmark helpers to use MSS. |
| src/tests/bench_bgra2rgb.py | Updates benchmark script to use MSS. |
| src/tests/bench_grab_windows.py | Updates Windows benchmark script to use MSS and _impl internals. |
| README.md | Updates quickstart snippet to from mss import MSS. |
| docs/source/index.rst | Updates documentation landing-page example to use MSS. |
| docs/source/usage.rst | Updates usage docs to prefer MSS and describe deprecated compatibility paths. |
| docs/source/examples.rst | Updates examples in docs to use MSS. |
| docs/source/conf.py | Adjusts doc build monkey-patch comment and removes unnecessary ignore. |
| docs/source/api.rst | Reduces API surface documented (aligning with new “single class” public API). |
| docs/source/examples/pil.py | Updates example to use mss.MSS. |
| docs/source/examples/pil_pixels.py | Updates example to use mss.MSS. |
| docs/source/examples/part_of_screen.py | Updates example to use mss.MSS. |
| docs/source/examples/part_of_screen_monitor_2.py | Updates example to use mss.MSS. |
| docs/source/examples/opencv_numpy.py | Updates example to use mss.MSS. |
| docs/source/examples/linux_xshm_backend.py | Updates example to MSS(backend=...) rather than backend-specific import. |
| docs/source/examples/linux_display_keyword.py | Updates example to use mss.MSS(display=...). |
| docs/source/examples/from_pil_tuple.py | Updates example to use mss.MSS. |
| docs/source/examples/fps.py | Updates example to use mss.MSS. |
| docs/source/examples/fps_multiprocessing.py | Updates example to use mss.MSS. |
| docs/source/examples/custom_cls_image.py | Updates example to use mss.MSS. |
| docs/source/examples/callback.py | Updates example to use mss.MSS. |
| demos/video-capture.py | Updates demo typing and construction to mss.MSS. |
| demos/video-capture-simple.py | Updates demo to mss.MSS. |
| demos/tinytv-stream.py | Updates demo to mss.MSS. |
| demos/tinytv-stream-simple.py | Updates demo to mss.MSS. |
| demos/cat-detector.py | Updates demo to mss.MSS. |
| CHANGES.md | Notes the new mss.MSS API and strategy refactor in release notes. |
| CHANGELOG.md | Notes the new mss.MSS API and compatibility story in changelog. |
Comments suppressed due to low confidence (1)
src/mss/linux/init.py:52
mss.linux.mss()appears to have been removed and replaced bychoose_impl(). Prior docs/tests referencedmss.linux.mssas part of the GNU/Linux backend-selection API, so dropping it outright can break existing user code. Consider reintroducingmss.linux.mss()as a deprecated wrapper (emittingDeprecationWarning) that constructs/returnsmss.MSS(backend=...)(or equivalent), while keepingchoose_impl()internal.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| **kwargs: Any, | ||
| ) -> None: | ||
| # TODO(jholveck): #493 Accept platform-specific kwargs on all platforms for migration ease. Foreign kwargs | ||
| # are silently stripped with a warning. | ||
| platform_only: dict[str, str] = { | ||
| "display": "linux", | ||
| "max_displays": "darwin", | ||
| } | ||
| os_ = platform.system().lower() | ||
| for kwarg_name, target_platform in platform_only.items(): | ||
| if kwarg_name in kwargs and os_ != target_platform: | ||
| kwargs.pop(kwarg_name) | ||
| warnings.warn( | ||
| f"{kwarg_name} is only used on {target_platform}. This will be an error in the future.", | ||
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) | ||
|
|
||
| self._impl: MSSImplementation = _choose_impl( | ||
| backend=backend, |
There was a problem hiding this comment.
The public mss.MSS constructor now only exposes backend and compression_level in its signature and relies on **kwargs for documented parameters like with_cursor, display, and max_displays. This weakens type checking and introspection (e.g., help(mss.MSS) / IDE completion) and makes it harder to treat mss.MSS as the stable API surface promised by this refactor. Consider adding these documented kwargs explicitly to the MSS.__init__ signature (still optionally accepting extra **kwargs for the transition/deprecation behavior) so the stable API is reflected in typing and docs.
| **kwargs: Any, | |
| ) -> None: | |
| # TODO(jholveck): #493 Accept platform-specific kwargs on all platforms for migration ease. Foreign kwargs | |
| # are silently stripped with a warning. | |
| platform_only: dict[str, str] = { | |
| "display": "linux", | |
| "max_displays": "darwin", | |
| } | |
| os_ = platform.system().lower() | |
| for kwarg_name, target_platform in platform_only.items(): | |
| if kwarg_name in kwargs and os_ != target_platform: | |
| kwargs.pop(kwarg_name) | |
| warnings.warn( | |
| f"{kwarg_name} is only used on {target_platform}. This will be an error in the future.", | |
| DeprecationWarning, | |
| stacklevel=2, | |
| ) | |
| self._impl: MSSImplementation = _choose_impl( | |
| backend=backend, | |
| with_cursor: bool | None = None, | |
| display: bytes | str | None = None, | |
| max_displays: int | None = None, | |
| **kwargs: Any, | |
| ) -> None: | |
| # TODO(jholveck): #493 Accept platform-specific kwargs on all platforms for migration ease. Foreign kwargs | |
| # are silently stripped with a warning. | |
| # Normalize documented keyword arguments that may have been passed via **kwargs | |
| if with_cursor is None and "with_cursor" in kwargs: | |
| with_cursor = kwargs.pop("with_cursor") | |
| if display is None and "display" in kwargs: | |
| display = kwargs.pop("display") | |
| if max_displays is None and "max_displays" in kwargs: | |
| max_displays = kwargs.pop("max_displays") | |
| os_ = platform.system().lower() | |
| # Enforce platform-specific options, preserving existing deprecation behavior. | |
| if display is not None and os_ != "linux": | |
| warnings.warn( | |
| "display is only used on linux. This will be an error in the future.", | |
| DeprecationWarning, | |
| stacklevel=2, | |
| ) | |
| display = None | |
| if max_displays is not None and os_ != "darwin": | |
| warnings.warn( | |
| "max_displays is only used on darwin. This will be an error in the future.", | |
| DeprecationWarning, | |
| stacklevel=2, | |
| ) | |
| max_displays = None | |
| self._impl: MSSImplementation = _choose_impl( | |
| backend=backend, | |
| with_cursor=with_cursor, | |
| display=display, | |
| max_displays=max_displays, |
See also BoboTiG/python-mss issue BoboTiG#486.
The user will always work with a single class:
mss.MSS. Differences in implementation, such as platform or capture strategy, are hidden in an internal implementation object held by the mss.MSS object.This allows us to change the implementation, with arbitrary class hierarchies, without worrying about preserving compatibility with internal class names.
This deprecates the existing
mssfactory function, although it can easily be kept for as long as needed to give users time to adapt.It also deprecates the existing
mss.{platform}.MSStypes. These are exposed to the user, so somebody callingmss.{platform}.MSS()in 10.x can still reasonably expect to get amss.{platform}.MSSobject back. However, in 11.0, we can remove the type entirely, and either remove those symbols, or make them deprecated aliases formss.MSS.Where possible, deprecated functionality emits a
DeprecationWarning. However, note that these are ignored by default, unless triggered by code in__main__.Many of the API docs are removed, since this change removes much of the API surface. However, they are still in available for backwards-compatibility.
This change adds tests for everything that was in the 10.1 docs, examples, etc, at least at a basic level: for instance, it tests that
mss.linux.MSSstill works as both a constructor and a type (forisinstance), and thatmss.linux.ZPIXMAPstill exists (it was listed in the 10.1 docs).The existing code, tests, and docs are changed to use
mss.MSS.