Move class definitions out of __init__.py files

[Yepoleb]

Code in __init__.py is horrible to find, so I suggest we keep it to the absolute minimum. Preferably imports only, but there may be some cases where actual code is necessary. Not a PR, because I want to hear your opinions first.

[Holiverh]

Can you elaborate on your thinking a bit for me? 😄

But also, I don't know if you saw, but in #49 I mentioned wanting to rearrange/flatten the package structure. It might coincide nicely with this?

[Yepoleb]

Yesterday it took me some time to find BaseQuerier, because I expected to find it in a file called basequerier.py or something similar. Looking in __init__.py was only my last resort when I couldn't find it in any other files.

[Holiverh]

Note: Initially I was going to add this as a comment to #56. But what I ended up writing was more speculative than I originally planned, so perhaps it makes more sense here.

I see little justification for not having common code directly at the package level, in __init__. The reference to valve.source.BaseQuerier is entirely unambiguous as to the location of BaseQuerier. I also do not believe this to module structure to be in violation of reasonable conventions and expectations. The main argument I see against __init__-level common code is if you want to use __init__ to rewrite the package structure, by importing things from modules within the package. That would inevitably result in cyclical dependencies.

It we were to do that, this change is far more compelling. For example, valve/source/__init__.py becomes something like:

from valve.source.a2s import ServerQuerier
from valve.source.master_server import MasterServerQuerier
from valve.source.util import (
    Platform,
    ServerType,
)

BaseQuerier and the various bits of message parsing (which I'm aware there is a separate proposal for) remain strictly internal to the valve.source package. My objection to this kinda of aliasing is that it violates the principal of having only one, obvious location for the API. For example, a user's code should only import from one location -- whereas with the approach described here, both import valve.source and import.source.master_server would make the same functionally accessible. This ambiguity may lead to confusion, especially as to whether the distinction between the two options is actually meaningful.

I'm seen solutions to this in which all package modules are marked as private, using the well understood leading underscore convention. Then the public API is imported into package's __init__. This sends a more significant signal to the user as to where the public API should be imported from.

There can be downsides to this. Mostly to do with the original module location leaking through in stack traces and such.

In summary, the package structure would become something like:

valve/
  __init__.py
  source/
    __init__.py
    _a2s.py
    _master_server.py
    _messages.py
    _util.py

As a separate consideration, it may also make sense to move MasterServerQuerier and ServerQuerier move to a common module, e.g. valve.source.query. Naturally, BaseQuerier could move with them. This provides far more obvious naming. valve.source.query provides tools for querying Source servers.

Exposing the name "A2S" to the user as part of the package structure seems bad. A2S is an implementation detail. An important one, that is worthy of discussion in the documentation. A user coming to Python-valve from no experience with this stuff is surely going to be asking themselves "what on Earth is a2s"!?

Returning to the initial discussion of creating a valve.source.query. If both the master server and source server querying interfaces lived within the same module, does it not make sense for the message serialisation to also live there? What I'm trying to explore is whether it's plausible that the entire valve.source package could be collapsed to a single module? E.g. becoming import valve.source or import valve.query for access to all Source server querying functionality. I don't really foresee new adding new functionality that wouldn't fit under the broad umbrella sub-package valve.source better than it would in a more specifically named, top-level module.

[Yepoleb]

For me having two import locations has never been something I questioned. Almost all packages I know have one file where an object is defined and another where it's imported for convenient access. Marking the files with an underscore is only necessary if you don't consider them a public interface.

Dividing the code into new sub-packages is always possible, I don't see a reason to add valve.source.query before there's a need to. Using __init__.py imports we can keep the module backwards compatible as long as we want.

The current structure is not necessarily wrong, it's just not very pythonic. Imports like tf.servers.valve.source.a2s.ServerQuerier are fine in the Java world, but in Python most packages have just a single layer where the entire public interface can be imported from. Maybe it's easiest if we just choose a well known Python module like requests and use their structure as a template.