Missing __init__.py file in a package results in omitting all subpackages and submodules

Issue #566 closed
Andrei Fokau
created an issue

PEP 420 allows one to skip creating empty __init__.py files in every package.

Coverage.py omits packages without __init__.py and therefore provides incorrect coverage stats.

Comments (17)

  1. Ned Batchelder repo owner

    I see: the "source=testme" setting makes coverage.py look in testme for files that could have run. It relies on __init__.py files to understand what is importable (as opposed to all the .py files in a scripts/ directory, for example, which are not).

    I'm not sure what the right behavior should be here. I'd like not to report about every .py script. Is there some other way to know that these files are meant to be imported?

  2. Andrei Fokau reporter

    @Ned Batchelder If I would use coverage.py for the first time, then I would expect that everything under source location is considered as source files and that coverage is calculated for all *.py files there, i.e. source/**/*.py. In case some files should be omitted, then I would specify them in the omit parameter, e.g. scripts/ directory. I would not expect coverage.py to skip subpackages and submodules just because I decided I don't need empty __init__.py files everywhere.

    As my example shows, coverage.py currently misses some perfectly importable subpackages and misinforms me about coverage in my project. This is a problem for e.g. a Django project management command which I would normally create as myproject/myapp/management/commands/mycommand.py without even thinking that I still need __init__.py in Python 3.3+.

    I believe this is an important issue.

  3. Andrei Fokau reporter

    unittest.TestLoader skips tests under PEP-420 packages which makes coverage.py one of the tools able to report about missing tests. So it's really very important that it finds all *.py files missing coverage!

  4. Andrei Fokau reporter

    @Ned Batchelder Oh, I think there are actually a lot more issues with the module discovery. Just found out that coverage.py does not check if directory names are valid identifiers:

    Name                                          Stmts   Miss Branch BrPart  Cover   Missing
    -----------------------------------------------------------------------------------------
    testme/__init__.py                                2      0      0      0   100%
    testme/invalid-name/__init__.py                   0      0      0      0   100%
    testme/invalid-name/invalid-sub/__init__.py       0      0      0      0   100%
    testme/invalid-name/invalid-sub/testme.py         2      2      0      0     0%   3-4
    testme/normal/__init__.py                         0      0      0      0   100%
    testme/normal/sub/__init__.py                     0      0      0      0   100%
    testme/normal/sub/testme.py                       2      2      0      0     0%   3-4
    -----------------------------------------------------------------------------------------
    TOTAL                                             6      4      0      0    33%
    

    Valid identifier definition is different for py2 and py3 due to unicode additions in PEP 3131.

    Things are even more complicated since ./testme/invalid-name/invalid-sub may be in sys.path so we should check that as well and that's not as straightforward as it may seem considering partial paths that can be there as well.

    Also, sys.path may contain other path (say ./testme/normal/sub/) with testme.py module before ./testme/invalid-name/invalid-sub so testme.py in the later won't be imported anyway. The question is -- should it then be included in the report? Maybe in a separate category, e.g. non-importable files, so that one sees that there are such -- can be very handy.

    One may want to give up on all these edge cases just to keep things simple, however, I consider coverage.py as vital as unittests themselves and strongly believe that it should cover all edge cases to be as reliable tool as possible.

  5. Andrei Fokau reporter

    I think the right thing to do is to report about all *.py files under source path, no matter if they are PEP 420 or have invalid identifiers in the path, and let the user omit them if they should be excluded. It will help to keep it simple and also cover all edges.

    It should behave the same way for Python 2 as well.

    If the users want, we could add a setting to enable importability check, although such check may be complicated. Non-importable files should be reported in that case.

  6. Ned Batchelder repo owner

    @Andrei Fokau This is getting complicated. I tend to think you are right that coverage.py needs to stay simple and let people override its logic for the more complicated cases. If we change it as you suggest, it will be an incompatible change, which makes it a bigger deal.

  7. Andrei Fokau reporter
    • Coverage.py not reporting about namespace packages is a bug which needs fixing.
    • Coverage.py reporting about all *.py files (even with invalid namespace) is an important feature.
    • Both are fixed by removing __init__.py check, although I need a bit more time for testing this properly.
  8. Ned Batchelder repo owner

    This is a breaking change from previous behavior. Another way to support it would be to let the user specify which are the 420 namespace directories, or perhaps to indicate that they want the new behavior of turning off all the name checking instead.

    Do you have a real project that this is impacting? I don't know how people are going to put PEP 420 to use. The recommendation is still to use __init__.py files in packages.

  9. Andrei Fokau reporter

    @Ned Batchelder Yeah, a setting would be the best solution to this. I am currently jumping between several private Django projects, one of which is using Python 3 where I do want to get rid of dosens of empty __init__.py files. Django does rely on __init__.py itself and through unittest, but this can be fixed by patching/extending the test runner etc. Coverage is run separately and it may be possible to patch it as well in a custom runner. A setting to control it would help for sure. I am working mostly with other projects right now and will come back to this issue as soon as I am done with them.

  10. Ned Batchelder repo owner

    One thing you should consider: PEP 420 isn't meant for you to be able to omit __init__.py files in every package. The point was to make namespace packages easier. If you aren't making a namespace package, then you should continue to include the __init__.py files. I understand that some find the empty files distasteful, but they are still the right way to make non-namespace packages.

  11. Log in to comment