Add lib.interface / qualifiers

[Finistere]

Better implementation of interface/implementation duality with predicates.

Public API:

from antidote import implements, inject, interface, world, QualifiedBy

V1 = object()
V2 = object()

@interface
class Service:
    pass

@implements(Service).when(qualified_by=V1)
class ServiceImpl(Service):
    pass

@implements(Service).when(QualifiedBy(V2))
class ServiceImplV2(Service):
    pass

world.get[Service].single(qualified_by=V1)
world.get[Service].all()

@inject
def f(service: Service = inject.impl(QualifiedBy(V2))) -> Service:
    return service

@inject
def f(services: list[Service] = inject.impl(qualified_by=[V1, V2])) -> list[Service]:
    return services

Experimental Predicate API:

from __future__ import annotations

from dataclasses import dataclass
from typing import Any, Optional

from antidote import implements, interface, QualifiedBy, world
from antidote.lib.interface import Predicate

@dataclass
class Weight:
    value: float

    @classmethod
    def of_neutral(cls, predicate: Optional[Predicate[Any]]) -> Weight:
        if isinstance(predicate, QualifiedBy):
            # Custom weight
            return Weight(len(predicate.qualifiers))
        return Weight(0)

    def __lt__(self, other: Weight) -> bool:
        return self.value < other.value

    def __add__(self, other: Weight) -> Weight:
        return Weight(self.value + other.value)

@dataclass
class LocaleIs:
    lang: str

    def weight(self) -> Weight:
        # Higher weight than 0, because if the language matches it should be prioritized over the
        # alternative implementations.
        # English has a lower weight because it always matches, as the default, and other languages
        # should be able to override it.
        return Weight(1000 if self.lang != 'en' else 500)

    def evaluate(self, predicate: Optional[LocaleIs]) -> bool:
        if predicate is None:
            # If no language is specified, there's just no need for it.
            return True

        return self.lang == predicate.lang or predicate.lang == 'en'

@interface
class Alert:
    def text(self) -> str:
        pass

@implements(Alert).when(LocaleIs('fr'))
class FrenchAlert(Alert):
    def text(self) -> str:
        return "Quelque chose en français!"

@implements(Alert).when(LocaleIs('en'))
class DefaultAlert(Alert):
    def text(self) -> str:
        return "Something in English!"

assert world.get[Alert].single(LocaleIs("fr")) is world.get(FrenchAlert)
assert world.get[Alert].single(LocaleIs("it")) is world.get(DefaultAlert)
assert world.get[Alert].single(LocaleIs("en")) is world.get(DefaultAlert)

EDIT: used a dataclass for the Weight.

[codecov-commenter]

Codecov Report

Merging #36 (552ef4a) into master (d2e8559) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            master       #36    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           52        62    +10     
  Lines         2790      3385   +595     
  Branches       497       608   +111     
==========================================
+ Hits          2790      3385   +595     

Impacted Files	Coverage Δ
src/antidote/world/test/_methods.py	100.00% <ø> (ø)
src/antidote/__init__.py	100.00% <100.00%> (ø)
src/antidote/_constants.py	100.00% <100.00%> (ø)
src/antidote/_factory.py	100.00% <100.00%> (ø)
src/antidote/_implementation.py	100.00% <100.00%> (ø)
src/antidote/_internal/__init__.py	100.00% <100.00%> (ø)
src/antidote/_internal/argspec.py	100.00% <100.00%> (ø)
src/antidote/_internal/dispatch.py	100.00% <100.00%> (ø)
src/antidote/_internal/utils/__init__.py	100.00% <100.00%> (ø)
src/antidote/_internal/utils/debug.py	100.00% <100.00%> (ø)
... and 25 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d2e8559...552ef4a. Read the comment docs.

[Finistere]

Resolves #33, resolves #32

[pauleveritt]

Let's say I did this: @implements(Alert). It would cover both of these cases, right?

assert world.get[Alert].single(LocaleIs("it")) is world.get(DefaultAlert)
assert world.get[Alert].single(LocaleIs("en")) is world.get(DefaultAlert)

[Finistere]

If you added a class like this:

@implements(Alert)
class NoLangAlert(Alert):
    def text(self) -> str:
        return "???"

when using the constraint LocaleIs("it"), the evaluate() method would be called with None. In this implementation, I choose to return True. So it would be a valid candidate and would be returned with world.get[Alert].all(LocaleIs("it")). However, with world.get[Alert].single(LocaleIs("it")) the result wouldn't change. The weight of an implementation with no predicate is determined by calling the of_neutral method of the weight class with None, which returns 0 in this case. Less than the other implementations:

from antidote.lib.interface import ImplementationsOf

# note: probably need a simpler way to expose this.
print(world.debug(ImplementationsOf(Alert).all()))

<∅> Interface Alert
├── [Weight(value=1000)] FrenchAlert
├── [Weight(value=500)] DefaultAlert
└── [Weight(value=0)] NoLangAlert

[Finistere]

Oh and if there only was a single implementation without predicate, yes it would be used because evaluate(None) returns True. As there would be no other implementations, the weight wouldn't matter.

[pauleveritt]

Here's a common case: overriding a built-in. Let's say I ship a pluggable app with this:

@implements(Alert)
class DefaultAlert(Alert):
    def text(self) -> str:
        return "The built-in alert"

Someone makes a local site with my pluggable app. But they don't like my Alert and want to replace it. For all cases -- no predicate:

@implements(Alert)
class DefaultAlert(Alert):
    def text(self) -> str:
        return "The built-in alert"

They'd like the "last" registered Alert. I tried doing world.get[Alert].single() but it requires some qualification. I tried world.get(Alert) and got:

  File "src/antidote/core/container.pyx", line 466, in antidote.core.container.RawContainer.get
antidote.core.exceptions.DependencyNotFoundError: examples.override.Alert

Looking at world.get[Alert].all() I see them in reverse order of registration, but not sure if that is stable.

I guess I expected world.get(Alert) to figure an override had happened.

[Finistere]

Interesting example! The order is stable, but it depends on the import order in your case since weights are equal which IMHO is bad idea. With the current implementation, you could handle it with a negative weight:

class NoAlternativeImplementations:
    def weight(self) -> Weight:
        return Weight(float('-inf'))

@implements(Alert).when(NoAlternativeImplementations())
class DefaultAlert(Alert):
    def text(self) -> str:
        return "The built-in alert"

@implements(Alert)
class CustomAlert(Alert):
    def text(self) -> str:
        return "The custom alert"

assert isinstance(world.get[Alert].single(), CustomAlert)

Off the top of my head Antidote could provide an override mechanism though like the following:

@implements(Alert).overridding(DefaultAlert)
class CustomAlert(Alert):
    def text(self) -> str:
        return "The custom alert"

I would explicitly require the implementation to override though as I don't want it to depend on the import order. It would replace the DefaultAlert in the same conditions. A second override on DefaultAlert would raise as it's not present anymore.

[Finistere]

It's also unclear to me for now whether world.get(Alert) should work or not. It would have the same behavior as world.get[Alert].single(). The latter is more explicit, but moving from a @service to @interface becomes painful.

[pauleveritt]

I'm likely to have a lot of comments on this branch. Do you want them here or in individual tickets?

[pauleveritt]

The .overriding is an interesting and useful DX. It could go even shorter: @overrides(Alert). Though I could see a second override as being useful. Imagine Sphinx defines a bundled Logo, and a theme overrides it with ThemeLogo. But in my local site, I replace with PaulLogo.

These aren't variations in different cases (predicates). It's all still a replace-in-all-cases.

[pauleveritt]

If you don't provide something like .overriding, perhaps you could add something like DefaultWeight but for negative values. Though more semantically accurate, like BaseWeight.

[pauleveritt]

In your NoAlternativeImplementations example, I tried writing a function to inject an Alert. This form works:

@inject
def warning() -> str:
    alert: Alert = world.get[Alert].single()
    return f'{alert.text()}!'

...but this doesn't:

@inject
def warning(alert: Alert = inject.me()) -> str:
    return f'{alert.text()}!'

[Finistere]

This should work:

@inject
def warning(alert: Alert = inject.impl()) -> str:
    return f'{alert.text()}!'

I'm currently making the distinction between interfaces and the rest, but I think it's a mistake. I'll redo a pass on it to make it as simple as possible and merge inject.me() and inject.impl().

The .overriding is an interesting and useful DX. It could go even shorter: @overrides(Alert).

At first sight I'll keep @implements(Alert).overriding(DefaultAlert), or equivalent, because:

• I need to know for which interface you're actually overriding DefaultAlert. I could track it, but I currently don't.
• I think that it's better in terms of DX. At least you know that every possible implementation is decorated with @implements(Alert) making it relatively easy to search for.

I'm likely to have a lot of comments on this branch. Do you want them here or in individual tickets?

Depends. :) If those are features, please open an issue, it'll be easier to track. For bugs or anything else that you find weird, you can put it here. I'm going to merge this PR in the coming days with some fixes and more documentation.

Thanks for all the feedback!

[flisboac]

@implements(Alert)
class CustomAlert(Alert):
    def text(self) -> str:
        return "The custom alert"

The syntax looks good. I just worry about the duplication there (repeating Alert twice). I suppose that, in the vast majority of cases, this single inheritance will be the norm, because most services may do a single job (if you follow, e.g. SRP, etc).

Considering that, we could check the decorated class' __mro__ and see if it has a single entry (or, at most, 2?). If it has, and it IS an abstract class, AND cls is not abc.ABC (because you can inherit abc.ABC to make a class abstract), then we could assume that the single superclass is the interface the user is implementing, without ambiguities. This verification could be done at decoration time, and we could raise an exception if conditions are not met, and then ask the user to be explicit about which interface he's providing.

Although this would look a bit weird:

@implements().when(NoAlternativeImplementations())
class DefaultAlert(Alert):
    def text(self) -> str:
        return "The built-in alert"

[pauleveritt]

@flisboac brings up an interesting point about @implements(Alert) and duplication in class CustomAlert(Alert).

From a type theory perspective, is the implementation a subclass -- a type of -- the interface? I thought it was a structural subtyping thing and thus the interface should not be inherited from.

[Finistere]

From a type theory perspective, is the implementation a subclass -- a type of -- the interface? I thought it was a structural subtyping thing and thus the interface should not be inherited from.

@interface / @implements support both Protocols and inheritance. It'll enforce the subclass if possible, so for inheritance and @runtime_checkable protocol. Unfortunately, it's runtime only. I lack the ability to specify constraints between type variables for static type checking...

The syntax looks good. I just worry about the duplication there (repeating Alert twice). I suppose that, in the vast majority of cases, this single inheritance will be the norm, because most services may do a single job (if you follow, e.g. SRP, etc).

I thought about that, my main issue with automatically detecting the interface is that it makes it harder to search for implementations. When forcing @implements(Alert), you can do a text-search for it and find all implementations easily, supposing you're not doing too much magic / using your own decorator. By @implements() you would need to rely on the IDE showing you the subclasses, and some of them may not be exposed to Antidote at all. It's not a big deal, but neither is writing Alert explicitly. So for now I'm leaning to the conservative side as it's easier to add than remove.

@flisboac, just in case as I don't find your comment anymore, the decorator expression can be easily used in Python 3.7 with this trick:

def _(x: T) -> T:
    return x

@_(implements().when(NoAlternativeImplementations()))
class DefaultAlert(Alert):
    def text(self) -> str:
        return "The built-in alert"

Or eventually by using your wrapper implements_when(...). It's not perfect, but to me, it feels good enouch.

[Finistere]

If you don't provide something like .overriding, perhaps you could add something like DefaultWeight but for negative values. Though more semantically accurate, like BaseWeight.

That's something I'd like to avoid. I'd like Antidote's own weight system to be as simple as just a neutral weight. This makes it not too painful to use your custom weight. You only need to support the neutral case. The complexer Antidote's system is, the harder it'll be to declare your own weights.

[flisboac]

(...) Or eventually by using your wrapper implements_when(...). It's not perfect, but to me, it feels good enouch.

That's a good trick, thanks! What I alluded to, in my deleted comment, was that I could simply alias the expression to a local variable, and use it to decorate whatever it is needed. But my comment overall was made because I was worried that Python 3.7 support was being dropped. I then quickly realised by looking at the PR that this was not the case, and deleted the comment as fast as I could. :p

[flisboac]

From a type theory perspective, is the implementation a subclass -- a type of -- the interface? I thought it was a structural subtyping thing and thus the interface should not be inherited from.

Well, the way I see it, it doesn't really matter.

Wherever an Alert is expected, any implementation of it could be used, if you follow LSP. On paper, interface implementation can be realised either by way of subclassing, or structural equivalence, the important factor being that it must provide and respect the interface's contract. The way the interface is described, or how the implementation is done, is limited or determined by what the language offers. (And Python offers a lot.)

From a static analysis point of view, both ways of implementing an interface (nominal and structural) work fine. The difference between them is more syntactic (more concerned with language peculiarities) than semantic (actually offering an implementation that respects the contract). (Btw, some type systems don't even make that distinction e.g. TypeScript; but some do, e.g. Java.)

Now, the advantage of nominal subtyping is that it's relatively trivial to check, both at static-time and runtime. For protocols and structural typing, things get a bit more difficult, or may incur more processing (ie. how to ensure @runtime_checkable, check if attributes exist one by one, etc). Antidote could abstract away the interface, but this would make dependency lookup a bit more difficult or cumbersome (because you need that information at runtime, and it's effectively missing, or should be given separately). That's why I think nominal subtyping has more value here.

Also, because you can replace one implementation with another, it's not far fetched to assume that there's a kind that's common to all of them, the interface. It could be a superclass of all implementations, and all conditions would still be met.

[flisboac]

I thought about that, my main issue with automatically detecting the interface is that it makes it harder to search for implementations. When forcing @implements(Alert), you can do a text-search for it and find all implementations easily, supposing you're not doing too much magic / using your own decorator. By @implements() you would need to rely on the IDE showing you the subclasses, and some of them may not be exposed to Antidote at all. It's not a big deal, but neither is writing Alert explicitly. So for now I'm leaning to the conservative side as it's easier to add than remove.

That's reasonable. And indeed, it's not a big deal. The fact that qualifiers are coming is already good enough! :D

[Finistere]

I was worried that Python 3.7 support was being dropped.

I'm keeping support for all Python versions that haven't reached their end-of-life yet, so for Python 3.7 it's until mid-2023 AFAIK.