Typing and public API

[bluetech]

pytest's "official" public API is exported by the pytest package, and everything else is defined in the _pytest package.

Currently, pytest only contains APIs that users need to use directly: functions to be called, decorators, classes to be instantiated or subclasssed, globals. Other objects, which the user doesn't interact with directly, are not exported.

Typing extends the range of cases in which some object needs to be referred to by name. As long as the user can somehow reach an object, directly or indirectly, through some public API, they need to be able to refer to its type by name, due to type annotations.

To come up with a list of reachable but unexported types, I used the following categories:

1. Fixtures: built-in fixtures that are dependency-injected. To benefit from the types the user needs to type annotate them, for example:

   def test_it(monkeypatch: MonkeyPatch) -> None:
       ...

2. Hooks: the user needs to be able to type annotate the arguments and return values of any hook.

3. Transitive: types that are referred to transitively by some other public API, e.g. return value of a function, method, context manager, public attribute etc.

See below for the list I came up with.

Questions for discussion:

1. Is this a problem we want to fix?

2. Most of these types are classes, that are not intended to be instantiated and/or subclassed by users, only internally.

   2.1. Do we want to enforce this?
   2.2. How do we enforce this?

3. How do we want to officialy declare something as public API?

4. Which types from the list below are we actually ready to export?

5. Is there a category or specific types I missed?

I'll add my own thoughts on these questions in a separate reply.

---

Fixtures:

• _pytest.fixtures.FixtureRequest -> pytest.FixtureRequest
• _pytest.fixtures.SubRequest
• _pytest.cacheprovider.Cache -> pytest.Cache
• _pytest.capture.CaptureFixture -> pytest.CaptureFixture
• _pytest.logging.LogCaptureFixture -> pytest.LogCaptureFixture
• _pytest.pytester.Pytester -> pytest.Pytester
• _pytest.pytester.Testdir -> pytest.Testdir
• _pytest.tmpdir.TempdirFactory -> pytest.TempdirFactory
• _pytest.tmpdir.TempPathFactory -> pytest.TempPathFactory
• _pytest.monkeypatch.MonkeyPatch -> pytest.MonkeyPatch
• _pytest.recwarn.WarningsRecorder -> pytest.WarningsRecorder

Hooks:

• _pytest.config.Config -> pytest.Config
• _pytest.config.PytestPluginManager -> pytest.PytestPluginManager
• _pytest.config.argparsing.Parser -> pytest.Parser
• _pytest.reports.CollectReport -> pytest.CollectReport
• _pytest.python.PyCollector (Made private Make PyCollector an implementation detail - don't use in hook type annotation #9264)
• _pytest.python.Metafunc -> pytest.Metafunc
• _pytest.runner.CallInfo -> pytest.CallInfo
• _pytest.reports.TestReport -> pytest.TestReport
• _pytest.fixtures.SubRequest
• _pytest.fixtures.FixtureDef
• _pytest.terminal.TerminalReporter
• _pytest._code.code.ExceptionRepr
• _pytest.code.ExceptionInfo -> pytest.ExceptionInfo

API:

• _pytest.mark.structures.ParameterSet

Transitive:

• _pytest.fixtures.FuncFixtureInfo (thru Metafunc)
• _pytest.fixtures.FixtureFunctionMarker
• _pytest.mark.structures.MarkGenerator -> pytest.MarkGenerator
• _pytest.mark.structures.MarkDecorator -> pytest.MarkDecorator
• _pytest.mark.structures.Mark -> pytest.Mark
• _pytest.mark.structures.NodeKeywords (thru Node) - only exposed as a MutableMapping instead.
• _pytest.code._code.TerminalRepr
• _pytest.python.CallSpec2
• _pytest.python_api.ApproxBase
• _pytest.python_api.RaisesContext - Not worth exposing, just a context manager.
• _pytest.recwarn.WarningsRecorder -> pytest.WarningsRecorder
• _pytest._io.TerminalWriter
• _pytest.config.argparsing.OptionGroup -> pytest.OptionGroup
• _pytest.pytester.ParsedCall -> pytest.RecordedHookCall
• _pytest.pytester.HookRecorder -> pytest.HookRecorder
• _pytest.pytester.RunResult -> pytest.RunResult
• _pytest.pytester.LineMatcher -> pytest.LineMatcher

[bluetech]

Here are my thoughts on the questions:

1. Is this a problem we want to fix?

Yes; otherwise users can't utilize types in these cases. Also, if we don't, people will be importing from _pytest anyway, which we don't want to happen.

More generally, I think it will be really good for us to clearly delineate what we consider public API and what is private. Currently it is somewhat murky which makes internal refactorings difficult.

2.1. Do we want to enforce this?

I think we want to enforce it, because otherwise instantiation and subclassing are implicitly assumed to be allowed, and then we are locked into supporting them even when we don't want to.

2.2. How do we enforce this?

My current intention is to steal trio's approach and see how it goes.

3. How do we want to officialy declare something as public API?

I think there should be three criteria:

1. It is exported in pytest.
2. It doesn't have a _ or __ prefix.
3. It is documented in the reference.

By "it" I mean both the types themselves, and their contents (methods, attributes, etc.).

Some work will be required to align reality with these requirements...

4. Which types from the list below are we actually ready to export?

The fixtures in the list look good to export, except maybe _pytest.fixtures.FixtureRequest / _pytest.fixtures.SubRequest which I'm not super clear about myself.

Stuff in the hooks needs to be exported; some require care however, and we might decide to leave unexported in the first pass.

Similar for the "Transitive" ones.

I think we can discuss the specifics in future PRs.

5. Is there a category or specific types I missed?

That's for others to answer :)

[nicoddemus]

Thanks @bluetech for writing up this issue with so much details. 👍

My thoughts:

Most of these types are classes, that are not intended to be instantiated and/or subclassed by users, only internally.

2.1. Do we want to enforce this?
2.2. How do we enforce this?

I think we want to enforce it, because otherwise instantiation and subclassing are implicitly assumed to be allowed, and then we are locked into supporting them even when we don't want to.

I agree that we don't want to allow instantiation and subclassing, specially because we might want to change __init__ methods later to include additional parameters to support new features.

An idea would be to implement declare ABCs, and only export the ABCs. This I believe would remove the need to export the actual type, and hence the __init__ method. It would also be a double-checking of the public API, because changing only the implementation would upset the type-checker and bring to light unintentional changes in the API.

The downside is that we would need to implement ABCs for all the objects you listed here, which is a bunch of work.

My current intention is to steal trio's approach and see how it goes.

This looks very similar to the work done by @RonnyPfannschmidt in the Node.from_parent endeavor.

By "it" I mean both the types themselves, and their contents (methods, attributes, etc.).

Those criteria look good to me!

[bluetech]

An idea would be to implement declare ABCs, and only export the ABCs

Doing this would be nice in that it would really highlight public vs. private. However I don't think we want to go this way for these reasons:

1. It doesn't prevent subclassing; in fact an ABC probably encourages it.
2. It requires a lot of duplication, scattered code and funny-looking "Impl" classes. I fear our code will look more like Java than Python :)
3. I'm not sure it's very practical because to gain the benefit we would need to to change all arguments/return values to the ABCs, but often our internal code uses public APIs and then would have to cast or such in order to access the private fields.
4. Not very newbie friendly.

[nicoddemus]

However I don't think we want to go this way for these reasons

All good points, I had a hunch it was a lot of work but hand't thought through the details, but you confirmed it. 😁

So I agree that ABCs is a no-go.

[RonnyPfannschmidt]

How about having a export module for types which has an explicitly spec that anything not exported somewhere else is only intended for type declaration and won't have public constructors for now

[gaborbernat]

An idea would be to implement declare ABCs, and only export the protocol definition. https://www.python.org/dev/peps/pep-0544/

What about using protocols instead of ABCs? And we only expose the protocols publicly 🤔 That could even be separated into a typing module:

from pytest.typing import MonkeyPatch, TempPathFactory

This way the fixtures have a very explicit public API, however the implementation does not have to inherit from it. Having the typing namespace makes it explicit that it's not meant for instiation. E.g.:

# pytest/typing.py

class Notset:
    def __repr__(self) -> str:
        return "<notset>"


notset = Notset()

K = TypeVar("K")
V = TypeVar("V")


class MonkeyPatch(Protocol):
    @contextmanager
    def context(self) -> Generator["MonkeyPatch", None, None]:
        ...

    @overload
    def setattr(self, target: str, name: object, value: Notset = ..., raising: bool = ...) -> None:
        ...

    @overload
    def setattr(self, target: object, name: str, value: object, raising: bool = ...) -> None:
        ...

    def setattr(
        self, target: Union[str, object], name: Union[object, str], value: object = ..., raising: bool = ...,
    ) -> None:
        ...

    def delattr(self, target: Union[object, str], name: Union[str, Notset] = notset, raising: bool = ...,) -> None:
        ...

    def setitem(self, dic: MutableMapping[K, V], name: K, value: V) -> None:
        ...

    def delitem(self, dic: MutableMapping[K, V], name: K, raising: bool = ...) -> None:
        ...

[asottile]

I like the idea of protocols as a public api, it gives us a little bit more flexibility on the implementation

the tricky parts become though:

(1) how to ensure we stay in line with the protocols (typing tests?)
(2) if we use the protocols internally, we'd probably have to cast to the concrete types occasionally to use the internal methods? this feels like it defeats some of the purposes of typing in the first place 🤔

[gaborbernat]

I don't think you need to cast protocols, they should be replaceable automatically. You can test it by merging the protocol and implementation via retype into a temporary folder 🤔and running type check on that 🤔

[asottile]

@gaborbernat the problem would be if we internally defined things like:

def internal_method(x: ProtocolType) -> ...
    x.other_internal_method()

that would not type check because other_internal_method is not on the public protocol