Move to a strategy implementation design.#494
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.
|
Thank you @jholveck, that's 👍 on my side! |
|
Terrific; glad to hear it! I'm going to leave it as draft for a day or two to make sure nothing else occurs to me, while giving you and @halldorfannar a chance to review. This refactor was the big thing that I had before wanting to release 10.2. But I do want to add the ScreenShot alias we discussed (simple), and maybe a docs section about the version compatibility policy. I'll do those in separate PRs, though. What's on your plate before a 10.2 release? |
|
By the way, here's a summary of the API surface from the 10.1 docs. I used this as a guide when looking at what to make sure was preserved and tested. 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 (
|
See also BoboTiG/python-mss issue #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.Changes proposed in this PR
Fixes #486
./check.shpassed