Skip to content

Move to a strategy implementation design#7

Closed
jholveck wants to merge 4 commits intomainfrom
strategy
Closed

Move to a strategy implementation design#7
jholveck wants to merge 4 commits intomainfrom
strategy

Conversation

@jholveck
Copy link
Copy Markdown
Owner

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.

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.
@jholveck
Copy link
Copy Markdown
Owner Author

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 summary

This issue’s goal is to support multiple backends while keeping the public MSS API stable for users.

The current Linux implementation changed mss.linux.MSS from a class into a constructor function in order to support multiple backends (Xlib, XCB GetImage, XCB ShmGetImage). However, this breaks the expectation that mss.linux.MSS is a class, which affects type compatibility and may surprise users.

In the future, other platforms will likely need similar backend selection. For example:

  • newer Windows capture APIs that are not available on all systems
  • a replacement for the deprecated macOS capture API
  • different APIs for full-screen vs. window capture
  • capture directly to GPU memory (CUDA)

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:

  • Introduce a public mss.MSS class that provides the user-facing API.
  • Backend implementations become internal strategy objects.
  • mss.{platform}.MSS becomes a thin subclass of mss.MSS for compatibility during the transition.
  • Backend selection happens inside mss.MSS during initialization.

Benefits of this approach:

  • Keeps a stable public class for users and type annotations.
  • Allows multiple backends per platform.
  • Clearly separates the public API from internal backend implementations.
  • Lets us reorganize backend code without breaking user-facing APIs.

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.


Details

I think I should change the way that I currently provide mss.linux.MSS.

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 backend parameter. Now, the instance it returns is not an instance of mss.linux.MSS; that’s not even a class anymore. This breaks the type compatibility guarantee that we started talking about in the Monitors change, and continued talking about in the Release Planning issue. I need to do something about that.

Longer-term, we don’t just have Linux to consider. The other platforms are likely to need parallel backends too:

  • New, faster Windows capture APIs that aren’t on all systems
  • New macOS capture API, since the one we use now is already deprecated
  • Anything that uses different APIs for full-screen capture vs. single-window capture
  • Anything that captures to CUDA, etc.

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 XShmGetImage isn’t available, it should use the XGetImage backend. Similarly, the choice of backend may also depend on user-specified flags: the choice of source (full-screen vs. window), destination (CPU or GPU), and maybe other considerations.

We don’t provide a generic mss.MSS class that the user can construct. That’s because it’s clearly a job for a factory function: it makes the decision of which class to return based on probes (platform.system). Similarly, with multiple backends on a given platform, it seems that it doesn’t make sense to say that they’re one class.

In the same way, in the future, mss.{platform}.MSS should not promise a specific class. Instead, the public API should promise a capture object that implements the MSS interface.


I see a few possible paths. I currently prefer the strategy pattern approach (Path 2).

Path 1: Factory functions

We remove the mss.{platform}.MSS entry point, or at least, provide just a mss.{platform}.mss factory function.

For 10.2.0, make mss.{platform}.MSS a small class that just stores a real backend object, and forwards methods to that:

class MSS(MSSBase):
    def __init__(self, /, *args, **kwargs):
        msg = "Instantiating mss.linux.MSS directly is deprecated, and will be removed in 11.0.  Use the mss.linux.mss factory function instead."
        warnings.warn(msg, DeprecationWarning)
        self._impl = mss(*args, **kwargs)

    def _grab_impl(self, monitor):
        return self._impl._grab_impl(monitor)

    # etc.

For 11.0, turn mss.{platform}.MSS into a function, to handle users who haven’t updated their code. There’s going to be a fairly short window between 10.2 and 11.0, I expect, so we may want to not break their code promptly. Put an obvious deprecation notice in the docs.

For 12.0 (whenever that is), remove mss.{platform}.MSS entirely. The name makes it seem like a class, and that is likely to be a point of confusion. I’d actually recommend we remove it entirely in 11.0, except that I think the deprecation period would be pretty small for such a significant change.

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 mss.mss factory, but if we have a reason to still provide mss.{platform}.mss factories, it’d be easier that way. We also would have to decide what concrete classes we want to commit to.

This would require some migration for users:

  • Users who type-annotate using mss.{platform}.MSS would need to change before using 11.0.
  • Users who don’t type-annotate, but still call mss.{platform}.MSS, would need to change before using 12.0.

Path 2: Strategy pattern

This 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: MSSBase is what provides grab, save, monitors, and other user-visible methods. It then uses _grab_impl, _cursor_impl, _monitors_impl, and _close_impl to do the platform- or backend-specific work.

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 mss.MSS class (which is essentially what’s in mss.base.MSSBase today), construct it directly, refer to it in type annotations, etc. It also lets us improve on all the backend code and implementation details, reorganize however we want, without worrying about breaking user-facing code.

In this plan, mss.MSS becomes the canonical type, and mss.base.MSSBase (if we even keep it) becomes an implementation detail.

# Today
sct = mss.linux.MSS()
type(sct) is mss.linux.XShmGetImage

# Proposal
sct = mss.MSS()
type(sct) is mss.MSS

Compatibility is easy. We’d need to have mss.{platform}.MSS be an empty subclass of mss.MSS for 10.2. That would let users can keep calling it and referring to that type. We’d probably keep that available through the 11.0 cycle. But that would be easy, and not complicate things.

I particularly like this option for a few reasons:

  • Backwards compatibility is trivial.
  • We can change the implementation radically as we continue to improve MSS, without worrying about a large API surface to support.
  • It makes the API surface very clear. Anything not on mss.MSS or mss.ScreenShot becomes private to us. Right now, for instance, we expose MSSBase as a type for annotation purposes, but it’s not something the user can instantiate directly. With this proposal, the boundary between “public API” and “internal details” becomes a sharp one.
  • It makes the documentation more clear. Right now, much of the Sphinx documentation is under MSSBase. But that’s not a name the user is going to ever interact with. Indeed, the docs end up just being the public surfaces, without all of the subclasses and so forth.

Path 3: Constructor as factory

I don’t like this one, but it’s a possibility.

We can provide a mss.{platform}.MSS nearly-empty class that overrides __new__ to return a concrete subclass. Essentially, we put the factory function in __new__ instead of in a normal function. All the backends then would inherit from mss.{platform}.MSS.

I think that using __new__ this way is more likely to be confusing in the long term than helpful. I can describe some specific disadvantages, if needed, but mostly it’s just a “bad smell” to me. It makes things unnecessarily convoluted.

@jholveck
Copy link
Copy Markdown
Owner Author

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 mss.linux.MSS - previously the class that was the only Linux implementation - be a factory function. But then I realized that this would break type declarations.

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 mss.linux.mss factory function that existed in main prior to this change; we can safely remove that, since it was never in a public release.

@jholveck
Copy link
Copy Markdown
Owner Author

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 Inventory

This is a pragmatic inventory of what users were likely relying on in 10.1.0, based on:

  • docs in docs/source/*.rst
  • shipped example code in docs/source/examples/*.py
  • obvious entry-point module docstrings/signatures in src/mss/*

This is not an exhaustive symbol dump. It is a compatibility checklist for 10.2 and 11.0 transition work.

Tag legend:

  • [api] means the item appears in docs/source/api.rst but is not reinforced elsewhere in docs/examples and is less likely to be widely relied on. Keep if practical, but generally lower preservation priority than untagged items.

1) Top-level package surface (mss)

Likely public imports from src/mss/__init__.py:

  • mss.mss (factory function)
  • mss.ScreenShotError (exception class)

Also visibly present and commonly inspected by users/tools:

  • mss.__version__

Import/call styles shown in docs/examples:

  • from mss import mss then with mss() as sct:
  • import mss then with mss.mss() as sct:
  • import mss.tools then mss.tools.to_png(...)

2) Factory and main object

Primary construction path (shown in README/docs/examples):

  • mss.mss(**kwargs) -> MSSBase (actually returns OS-specific subclass)

Documented/used kwargs:

  • compression_level: int = 6
  • with_cursor: bool = False
  • display: bytes | str | None = None (Linux-focused)
  • max_displays: int = 32 (macOS-focused)

Returned runtime class is one of:

  • mss.darwin.MSS
  • mss.linux.MSS
  • mss.windows.MSS

Compatibility note:

  • docs describe this only implicitly ("appropriate MSS class" + OS-specific import examples)
  • explicit concrete-class guarantee exists in implementation (src/mss/factory.py) rather than strongly worded prose docs
  • for 10.2/11.0 planning, preserving exact concrete return class identity can be treated as lower-priority than preserving MSSBase behavior

Direct OS-specific imports are documented in usage.rst and therefore likely public:

  • from mss.linux import MSS as mss
  • from mss.darwin import MSS as mss
  • from mss.windows import MSS as mss

3) Core screenshot session object API (MSSBase / platform MSS)

Documented module-level symbol in mss.base:

  • [api] lock (threading lock)

Public methods users are shown to call:

  • close() -> None
  • grab(monitor_or_bbox) -> ScreenShot
  • save(mon=0, output="monitor-{mon}.png", callback=None) -> Iterator[str]
  • shot(**kwargs) -> str

Public context-manager protocol users rely on:

  • __enter__() / __exit__() via with mss() as sct:

Public attributes/properties users are shown to access or set:

  • monitors (property): list[dict[str, int]]
  • cls_image (custom screenshot class hook)
  • compression_level (PNG compression control)
  • with_cursor

Monitor dictionary shape (implied contract throughout docs/examples):

  • keys: left, top, width, height

grab() accepted inputs (documented + examples):

  • monitor dict with the shape above
  • PIL-style bbox tuple (left, top, right, lower)

save() filename formatting contract (documented):

  • placeholders: {mon}, {top}, {left}, {width}, {height}, {date}

4) Returned image object API (mss.screenshot.ScreenShot)

grab() returns ScreenShot (or subclass if cls_image is overridden).

Public constructor/class APIs likely relied upon:

  • ScreenShot(data, monitor, *, size=None)
  • ScreenShot.from_size(data, width, height)

Public methods:

  • pixel(coord_x, coord_y) -> tuple[int, int, int]

Public properties/attributes (docs/examples use these directly):

  • raw (BGRA bytearray)
  • bgra (bytes)
  • rgb (bytes)
  • pixels (2D RGB tuples)
  • pos (named tuple with left, top)
  • size (named tuple with width, height)
  • left
  • top
  • width
  • height
  • __array_interface__ (NumPy interoperability)

raw usage note:

  • not code-only; it is documented in API docs (docs/source/api.rst) and used directly in shipped example code (docs/source/examples/pil_pixels.py)
  • it is also described in the ScreenShot entry-point class comments/docstrings (src/mss/screenshot.py)

High-confidence example usage includes:

  • np.array(sct.grab(...))
  • sct_img.rgb, sct_img.bgra, sct_img.raw, sct_img.size
  • sct_img.width, sct_img.height, sct_img.pixel(x, y)

5) Helper function API

Public helper used in docs/examples:

  • mss.tools.to_png(data, size, *, level=6, output=None) -> bytes | None

Behavior contract:

  • if output is None, returns PNG bytes
  • otherwise writes a PNG file and returns None

6) Exception API

Public exception:

  • mss.exception.ScreenShotError

Public attribute documented for user handling:

  • .details: dict[str, Any] (notably for X11 error details on Linux)

7) CLI surface (python -m mss / mss)

Documented in usage.rst and implemented in src/mss/__main__.py.

Likely compatibility-sensitive options:

  • -c, --coordinates
  • -l, --level
  • -m, --monitor
  • -o, --output
  • --with-cursor
  • -q, --quiet
  • -v, --version

Documented coordinate format contract:

  • top, left, width, height

8) Documented platform low-level symbols (niche but publicly documented)

Because these appear in docs/source/api.rst, they were at least documented as public, even if most users do not rely on them.

mss.darwin documented symbols:

  • [api] CFUNCTIONS
  • [api] cgfloat
  • [api] CGPoint, CGSize, CGRect
  • [api] class MSS attributes: core, max_displays

mss.linux documented symbols:

  • [api] CFUNCTIONS, PLAINMASK, ZPIXMAP
  • [api] Display, XErrorEvent, XFixesCursorImage, XImage, XRRCrtcInfo, XRRModeInfo, XRRScreenResources, XWindowAttributes
  • [api] class MSS method: close()

mss.windows documented symbols:

  • [api] CAPTUREBLT, CFUNCTIONS, DIB_RGB_COLORS, SRCCOPY, MONITORNUMPROC
  • [api] BITMAPINFOHEADER, BITMAPINFO
  • [api] class MSS attributes: gdi32, user32

9) Additional likely-public typing/data-model symbols

Examples/docs expose or encourage importing these:

  • mss.models.Monitor
  • mss.models.Monitors
  • mss.models.Pixel
  • mss.models.Pixels
  • mss.models.Pos
  • mss.models.Size

Note: these are type aliases / named tuples. They are lower risk to keep but still worth preserving if refactoring modules.

10) Documented vs runtime mismatches (worth preserving intentionally)

There are stale-doc mismatches in 10.1.0 that users may still have copied from docs.

save() defaults:

  • api.rst documents save([mon=1], [output='mon-{mon}.png'], ...)
  • runtime in src/mss/base.py is mon=0, output='monitor-{mon}.png'

shot() behavior:

  • runtime default is effectively monitor 1 (shot() sets mon=1 if absent)
  • examples/docs consistently present shot() as "monitor 1" behavior

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 to MSSImpl* implementations.
  • Adds/updates compatibility shims (deprecated mss.mss, deprecated mss.{platform}.MSS) and adds compatibility-focused tests.
  • Updates docs, demos, benchmarks, and tests to use mss.MSS directly.

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 by choose_impl(). Prior docs/tests referenced mss.linux.mss as part of the GNU/Linux backend-selection API, so dropping it outright can break existing user code. Consider reintroducing mss.linux.mss() as a deprecated wrapper (emitting DeprecationWarning) that constructs/returns mss.MSS(backend=...) (or equivalent), while keeping choose_impl() internal.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +175 to +194
**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,
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
**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,

Copilot uses AI. Check for mistakes.
@jholveck jholveck closed this Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants