Restrict any logics inside __init__.py

[sobolevn]

Side effect inside __init__.py are nightmares.

We need to restrict any kind of python code inside __init__.py except:

1. comments
2. docstrings

Imports should be forbidden. Each separate module should have its own public API.

[mcproger]

@sobolevn I would like to work on this issue.

[sobolevn]

Oh, great!

What I have in mind is:
0. We need to create new error for this types of issues, let it be Z400 (as I reserve Z3xx for classes)

1. We need to create ModuleFileVisiter to check these kinds of features
2. Then we need to pass the filename and the ast tree into this visiter
3. And then check only root-level nodes to be in whitelist (just docs for now)
4. Raise errors if other things will be there

Other things that will be required:

1. integration test
2. noqa example, since sometimes we really need this feature, and we need a way to disable the check for a single file

Thanks for your support!

[mcproger]

@sobolevn Sorry, but I have too much other work now. I can still work on this task, but I can't guarantee how soon it will be ready.

[sobolevn]

@mcproger ok

[malinoff]

What about importing submodules and populating __all__? This is a python code, for sure.

[sobolevn]

I prefer not to import any kind of stuff in __init__.py. This way I always use full path to import things.

The only possibility to use this hack is when you have a backwards compatibility with clients you don't have in control. This way you go with # noqa: ... and documentation.

When you have control over your code and moving things around, just refactor all imports and continue.

The same goes for __all__. All regular names are public by default, all underscored names are protected/private by default. So we do not use, and even forbid to use __all__.

[malinoff]

If you ever want somebody else to use the tool, you have to adopt current practices, one of them is to populate __all__ explicitly.

Maybe we can agree on something in between? For example, to have a check that __init__.py has less than X lines (say, 20 by default).

[sobolevn]

@malinoff can you please elaborate on why __all__ is important?

[malinoff]

I suppose you are looking for a technical argument, not philosophical one (like "explicit is better than implicit").
I have one for you. Consider the following snippet:

import os
import re
import subprocess

def call_some_command(cmd):
  # call the cmd
  # parse output with re
  # do filesystem manipulation

With __all__, only the call_some_command function is exported. Without __all__, as you said, every single name not starting with an underscore is exported which means call_some_command, os, re and subprocess.

I guess it's okay-ish if imported modules are easily recognizable for any python developer. But how are you going to distinguish your public apis from, say, specific 3rd party modules?

[sobolevn]

Please, keep in mind that import * is already forbidden.
So, there is no technical need in __all__ property.
Which is only required for these kinds of imports: https://docs.python.org/3/tutorial/modules.html#importing-from-a-package

And indeed, there's no way to distinguish our API from 3rd party modules, imports, etc.
And that's not really a problem for me.

If you really want to make some import protected you can alias it: from os import path as _path.
But, I have not seen any real world usecases for this.

[edmondop]

Is there a reason why using all is discouraged?

[sobolevn]

@edmondo1984 because it is basically used together with import *, which is not allowed.
We also ask people to use _protected convention over managing public / protected via __all__. So, it is not useful at all in our case.

[sobolevn]

If you find that docs do not explain this, please, feel free to send a PR 🙂

[edmondop]

How do you expose the public api of a package? Think about a package parquet with three files inside:

• init.py
• reader.py
• writer.py

You want to expose public api parquet.read_file and parquet.write_file so your __init__.py looks like so

from .reader import read_file
from .writer import write_file

__all__ = ['read_file', 'write_file']

[sobolevn]

from .reader import read_file as read_file
from .writer import write_file as write_file

so, this would be compatible with mypy's implicit_reexport rule.

[edmondop]

This fails with complaints :

WPS113 Found same alias import: read_file
F401  '. reader. read_file' imported but unused

Also, if you have a use case like so

from my_package import parquet
from my_package.parquet import reader

this fails,

but moving the second import as a relative one like so:

from my_package import parquet
from parquet import reader
 

would cause mypy to fail

[sobolevn]

Yes, you can disable WPS113 per all __init__.py files or per line.