Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make public & private interface more explicit #12246

Open
danieleades opened this issue Apr 9, 2024 · 8 comments
Open

make public & private interface more explicit #12246

danieleades opened this issue Apr 9, 2024 · 8 comments
Labels
type:proposal a feature suggestion

Comments

@danieleades
Copy link
Contributor

it is not totally obvious which methods and types in sphinx are part of the public interface. This makes accidental breaking changes very likely.

I propose the following convention for denoting public/private objects (for discussion). The convention should be adopted incrementally on a best-effort basis:

  • each module uses __all__ to exhaustively list its public API
  • private modules should use leading underscores in the module name
  • nested private modules need not use leading underscores
  • modules within sphinx may import objects from private modules, except
  • modules may not import objects from other modules which are not listed in __all__

Alternatives

  • leading underscores could also be used to denote module-private objects instead of an __all__ list

Enforcement

  • enable mypy no-explicit-reexport
@chrisjsewell
Copy link
Member

I think what you are describing is basically https://doc.rust-lang.org/reference/visibility-and-privacy.html 😄, but an "inversion" (i.e. public by default, instead of private), which is good 👍

each module uses __all__ to exhaustively list its public API

I would say that the use of __all__ is primarily to allow for re-exporting private objects (i.e. https://doc.rust-lang.org/reference/visibility-and-privacy.html#re-exporting-and-visibility)

and so I would say maybe __all__ is not necessary in a file (as it would be difficult to maintain),
but if it is present, then it should contain as a minimum all non _ objects present in that file,
plus any re-exported objects you want to expose to the public API.

i.e. you can't make things private via the the use of __all__ (that is what _ is for), only expose inner things

For example, this is good:

__all__ = ("a", "X", "function")

from .inner import function

def a(): ...
def _b(): ...
class X: ...

But this is not good:

__all__ = ()

def a(): ...
class X: ...

@danieleades
Copy link
Contributor Author

I think what you are describing is basically https://doc.rust-lang.org/reference/visibility-and-privacy.html 😄, but an "inversion" (i.e. public by default, instead of private), which is good 👍

not exactly- the use of __all__ (as opposed to the use of leading underscores) makes all objects in a module private by default, unless explicitly included in the __all__

each module uses __all__ to exhaustively list its public API

I would say that the use of __all__ is primarily to allow for re-exporting private objects (i.e. https://doc.rust-lang.org/reference/visibility-and-privacy.html#re-exporting-and-visibility)

actually i think it main use case was originally controlling the behaviour of glob imports. But it's now used also used for defining public interfaces and also explicit re-exports, as you mention

and so I would say maybe __all__ is not necessary in a file (as it would be difficult to maintain), but if it is present, then it should contain as a minimum all non _ objects present in that file, plus any re-exported objects you want to expose to the public API.

no, i don't think this is right. The point of using __all__ is to minimise the interface that is exposed and maximise encapsulation. As such it should not include all objects without leading underscores, only those which are explicitly intended to be imported elsewhere. Otherwise there's no point using __all__ and you can just use the PEP8 leading underscore convention to denote visibility

i.e. you can't make things private via the the use of __all__ (that is what _ is for), only expose inner things

For example, this is good:

__all__ = ("a", "X", "function")

from .inner import function

def a(): ...
def _b(): ...
class X: ...

But this is not good:

__all__ = ()

def a(): ...
class X: ...

@danieleades
Copy link
Contributor Author

by the way, i'm not suggesting my proposal is definitely the way to go- more a straw man to kick off the discussion

@chrisjsewell
Copy link
Member

chrisjsewell commented Apr 9, 2024

tactually i think it main use case was originally controlling the behaviour of glob imports. But it's now used also used for defining public interfaces and also explicit re-exports, as you mention

Well formally its still only for * imports: https://docs.python.org/3/tutorial/modules.html#importing-from-a-package,
anything else is an informal thing

not exactly- the use of all (as opposed to the use of leading underscores) makes all objects in a module private by default, unless explicitly included in the all

and that is why I feel it would not be practical to expect users to obey this, since it is not enforced (to my knowledge) by any IDEs, linters, etc

(By contrast, for exposing inner objects, mypy can enforce that: https://mypy.readthedocs.io/en/stable/config_file.html#confval-implicit_reexport)

@chrisjsewell
Copy link
Member

@electric-coder I think you missed the point of what I said; I specifically said you can use __all__ for re-exports, i.e. to make private objects public, but what you can't use it for is to make public objects private

@picnixz
Copy link
Member

picnixz commented Apr 10, 2024

Well formally its still only for * imports

Not entirely. PEP 8 explicitly states (ref):

To better support introspection, modules should explicitly declare the names in their public API using the __all__ attribute. Setting __all__ to an empty list indicates that the module has no public API.

However, they also say

Even with __all__ set appropriately, internal interfaces (packages, modules, classes, functions, attributes or other names) should still be prefixed with a single leading underscore.

As such,

i.e. you can't make things private via the the use of __all__ (that is what _ is for), only expose inner things

Technically, yes and no. One thing that I can say is that in private modules and their children, everything is considered private, whether there's an underscore or not. I personally am more leaning towards:

Documented interfaces are considered public, unless the documentation explicitly declares them to be provisional or internal interfaces exempt from the usual backwards compatibility guarantees. All undocumented interfaces should be assumed to be internal.

Now

and that is why I feel it would not be practical to expect users to obey this, since it is not enforced (to my knowledge) by any IDEs, linters, etc

Well... I don't want to make the whole code base fool-proof honestly. The main problem with Python is that there is no notion of a protected scope because when you have an _-prefixed method, you don't know whether it's private or something that can be overridden in subclasses for instance.

I can suggest something that could meet both my expectations and probably yours as well: let's divide the API into private and public modules where in private modules we have public names and those private modules are imported (or we can even import the public names as is because imports should always be considered an implementation detail).

@danieleades
Copy link
Contributor Author

Documented interfaces are considered public, unless the documentation explicitly declares them to be provisional or internal interfaces exempt from the usual backwards compatibility guarantees. All undocumented interfaces should be assumed to be internal.

I would hate for the presence of documentation to indicate the boundary between private and public. Documentation of private modules and objects could be very useful for contributors.

Is there an equivalent of rust's cargo doc --document-private-items?

@picnixz
Copy link
Member

picnixz commented Apr 10, 2024

Err, you can include private members using autodoc with :private-members: but since we generally put our doc manually in doc, it's not an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:proposal a feature suggestion
Projects
None yet
Development

No branches or pull requests

3 participants