Should there not be methods intended to be only called from inside of the package, but from the outside of the class they're defined in?

Question

_{Note: This is a follow-up to this question on StackOverflow.}

I have to write a wrapper in Python to an external API, accessible through HTTP. The code is supposed to be publicly available on GitHub.

For this reason, I thought, it would be nice if a person cloning this repository wouldn't see tons of warnings. I opened my own code in PyCharm just to see if that was the case. It wasn't.

However, since I'm afraid what these warnings are pointing at is a design issue, please let me start by showing (in a simplified way) the design I came with:

Firstly: There are two methods to authenticate the HTTP connection. For this reason, I have a ConnectionBase abstract class, that is implemented by two concrete classes, each using one of the two available authentication methods. So far so good.

But here problems start. There is an ApiClient class that is supposed to contain wrappers of all those API routes. This ApiClient class, of course, has to make use of an instance of ConnectionBase. In addition, since there are so many API calls to provide wrappers for, I thought I'd break them into categories.

Finally, this is how (roughly) the definition of ApiClient looks like:

class ApiClient:
    def __init__(self, connection):
        self._connection = connection

    # forwarding method
    async def _make_call(*args, **kwargs):
        return await self._connection._make_call(*args, **kwargs)

    class _Category:
        def __init__(self, client):
            self._client = client

        # another forwarding method
        async def _make_call(*args, **kwargs):
            return await self._client._make_call(*args, **kwargs)

    class _SomeCategory(_Category):
        async def some_api_call(some_arg, other_arg):
            return await self._make_call(blah blah)

        async def other_api_call(some_arg, other_arg):
            if some_arg.some_condition():
                other_arg.whatever_logic_here()
            return await self._make_call(yadda yadda)

    class _OtherCategory(_Category):
        async def yet_another_api_call(some_arg, other_arg, yet_another_arg):
            #...
            return await self._make_call # etc etc

    @property
    def some_category(self):
        return ApiClient._SomeCategory(self)

    @property
    def other_category(self):
        return ApiClient._OtherCategory(self)

In this fashion, assuming that the user of my lib wants to make a call to this external API and that client is an instance of ApiClient, they would type:

client.some_category.some_api_call(some_arg, other_arg)

I believe that my use of underscores is clear: I'm preceding with an underscore all names that are not meant to be called by the end user of my lib. I thought this was the most important distinction: far more important than the distinction between variables private to a class: because the latter is nothing but an aesthetic issue, while the former is a usability issue: after all, exposing a (hopefully) clean, well-defined and intuitive public API is the very purpose of writing libraries!

Yet, PyCharm frequently complains that I'm accessing protected members of classes from outside of these classes. Apparently, and according to the linked SO question, the Pythonic understanding of a name preceded by an underscore is: This member is internal to this class and NOT the way I was understanding it, that is: This member is internal to this package.

So, all such lines are producing warnings:

await self._connection._make_call

await self._client._make_call

etc etc in other places of my code.

But then: Am I supposed to clearly distinguish between methods supposed to be called by the user of my lib and methods supposed to be called only from within my lib (or users who know the internal workings of my package and know what they're doing)?

If yes, then how if not by an underscore, which apparently means a different thing?

Or...

Well I don't know, because as it is clear from my questions on this site, my knowledge of design patterns is underwhelming... But a hypothesis of how this situation should be understood rises up in my mind... A pretty radical hypothesis...

Maybe the correct interpretation is that there should be no methods that are intended to be called from outside of a class but not from outside of the package and/or by the end user?

I mean, I'm constantly seeing talks about the need to break dependencies, about how bad it is to have code that is so closely intermingled and tightly coupled... Perhaps such methods are examples of this unwanted coupling? And in all cases where I'd like to put an underscore in the beginning of a method name but PyCharm complains, this simply means that the existence of such a method constitutes a coupling in my code that should not be there and therefore there should not be such a method at all?

And I was so proud of myself that I made use of dependency injection :P

On a more serious note: I have strong doubts if the above, dire interpretation is correct. In Uni I had a basic course on OO design. The instructor said something along the lines of, IIRC:

Please do remember that inheritance should be used in case of an is-a relationship, not a has-a relationship. An example of an extremely bad inheritance is a car inheriting from a wheel or from a gas pedal. Such cases should be handled by composition instead.

So, since an API client is not a connection used by this API client and since a category of an API call is not a client, I thought that it would be wrong to make API client inherit from a connection or to make a category inherit from the client - even though doing this would make PyCharm stop complaining and issuing warnings. Instead I used composition. However, if composition is used, I can't see how can I get rid of methods that are intended to be called from within my lib, but not from within the class they're defined in and not by the end user.

I suppose there must be a basic design principle I'm ignoring out of ignorance.

Could you enlighten me please?

Answer 1

With the below structure you could still use the call you provided of client.some_category.some_api_call(some_arg, other_arg)

Here it is:

class _Category:
    def __init__(self, client):
        self._client = client

    # another forwarding method
    async def _make_call(*args, **kwargs):
        return await self._client._make_call(*args, **kwargs)

class _SomeCategory(_Category):
    async def some_api_call(some_arg, other_arg):
        return await self._make_call(blah blah)

    async def other_api_call(some_arg, other_arg):
        if some_arg.some_condition():
            other_arg.whatever_logic_here()
        return await self._make_call(yadda yadda)

class _OtherCategory(_Category):
    async def yet_another_api_call(some_arg, other_arg, yet_another_arg):
        #...
        return await self._make_call # etc etc

class ApiClient:
    def __init__(self, connection):
        self._connection = connection
        self.some_category = _SomeCategory()
        self.other_category = _OtherCategory()

    # forwarding method
    async def _make_call(*args, **kwargs):
        return await self._connection._make_call(*args, **kwargs)

Answer 2

To start, PyCharm's warning about accessing a "protected" member is naive. If it sees the underscore prefix, it considers it to be "protected" and doesn't consider uses by first party code different than third party code. So that is just something you will need to deal with when developing a package and not wanting to expose the entire implementation.

---

As far as the example you presented, it seems like you artificially restricting your implementation to be based only on classes. Instead, module level functions can be used in place of class methods by passing in the values that need to be operated on.

This is a basic example, but could be expanded for the actual use case.

async def _make_connection_call(connection, *args, **kwargs):
    return await connection.make_call(*args, **kwargs)

 class _SomeCategory:
        def __init__(self, connection):
            self._connection = connection

        async def some_api_call(some_arg, other_arg):
            return await _make_connection_call(self._connection, some_arg, other_arg)

With this, the _Category is no longer needed and the function can't be leaked with the class when the instance is accessible by third party code.

Classes work well for things when state needs to be maintained. However, when the code simply needs a reference to X because X is managing some state, it can sometimes be better to pass the instance of X in as an argument instead of creating a class in order to keep the instance of X around.

---

In general, when the "protected" marker is set at a higher level, it doesn't also need to be used at the lower level. For example, if a library has a package named _internal all of the code within that doesn't also need the protected marker. When doing this, you do still need to be careful about what is exposed as part of the public API.

Another approach would be to use type hints to define Protocols (like an interface in a statically typed language). Then the public API would be annotated in terms of those public interfaces even if the actual returned instance has additional methods. This is still Python, so those methods would still technically be accessible. But PyCharm/MyPy would mark them incorrect if code using the library tried to use them.