Typechecking appears broken due to off-by-one relative package imports

[CAM-Gerlach]

Describe the bug

PRAW includes respectable coverage type annotations where practical (particularly given its dynamic nature), which have the potential to be very helpful to downstream developers seeking to catch many bugs statically without having to hit the live Reddit API, as well as PRAW development itself.

PRAW modules that use relative imports include the following block, presumably to help the type checker find the parent package (example from praw/reddit.py):

if TYPE_CHECKING:  # pragma: no cover
    from .. import praw

...with the number of .. increasing with each level deep the given module is in the package. However, this results in the relative import going outside the package, which is neither valid Python (without PEP 420 namespace packages) nor works with MyPy, as it means it recurses into a directory that is not itself a package. In addition, one file (praw/objector.py) has an extra .. (the only file that did not consistently follow this pattern), either due to a typo or being previously located in a subpackage.

The net result is that running mypy (and presumably other type checkers) on the PRAW package, or on user code importing praw after adding a py.typed file to the PRAW package root per PEP 561, will fail with a critical error and prevent type checking entirely, as the . Furthermore, this not only prevents type-checking what is often the most critical code in a Reddit-bot or client (along with PRAW's own correctness), but also causes stubgen to fail with a similar error, so users can't even provide their own stubs.

To Reproduce
Steps to reproduce the behavior:

1. Install PRAW and MyPy
2. Run stubgen -p praw -> Error occurs
3. Run mypy $SITE_PACKAGES/praw -> Error occurs
4. Run touch $SITE_PACKAGES/praw/py.typed to enable type-checking for PRAW and then mypy $YOUR_PACKAGE on a local package that users PRAW -> Error occurs

Expected behavior

Type checking to work without errors. To fix this issue, the relative imports for TYPE_CHECKING only can be simply changed to absolute imports (import praw), which are shorter, simpler and other than avoiding the errors/invalid behavior, have the exact same intended effect. Because all of these blocks are protected by if TYPE_CHECKING, there is zero runtime impact, it just fixes the type checker.

In addition, to allow PRAW users to take advantage of PRAW's existing type hints to type check their own code, I advise per PEP 561 adding an empty file named py.typed to the root of the package.

I've manually hacked it in there, and it provided helpful type hinting with no major issues and minimal false positives . However, to make this work for other devs locally and on our CIs, we have to run a script that implements these same hacks, which is fragile, ugly and has the potential to break the install. Fixing the import statements will alleviate 90% of that (we'll just have to touch one file), and adding the py.typed will do the rest.

I'm happy to submit a PR to fix this; let me know and I'll go ahead. I'd also be interested in cleaning up the remaining flagged typing inconsistencies in PRAW and getting it set up to continually check for bad types (e.g. with pre-commit, either locally or CI-only) so there isn't a regression on this front. I'd also hope to help add typing to prawcore, if that's desirable, as it currently seems to be missing at least some of it. Thanks!

Code/Logs

Error with mypy $SITE_PACKAGES/praw:

$SITE_PACKAGES/praw/models/base.py:6: error: No parent module -- cannot perform relative import
Found 1 error in 1 file (errors prevented further checking)

Same error with stubgen -p praw:

Critical error during semantic analysis: $SITE_PACKAGES/praw/models/base.py:6: error: No parent module -- cannot perform relative import

System Info

• OS: Windows 8.1 x64, Ubuntu 20.04 LTS, Raspbian 10
• Python: 3.7, 3.8, 3.9
• PRAW Version: 7.2.0, 7.3.0, latest master
• MyPy Version: 0.902, 0.910 (latest as of when I tested)

[LilSpazJoekp]

A couple things with this:

1. Does fixing the import in praw/objector.py seem to fix anything?
2. Would removing a level off of the other imports still allow pre_push.py to run successfully (more specifically, will docs still build correctly)?

[CAM-Gerlach]

Thanks for the quick reply!

Does fixing the import in praw/objector.py seem to fix anything?

Nope, for the reasons discussed above, though to be pedantic I tested it to confirm.

2. Would removing a level off of the other imports still allow pre_push.py to run successfully (more specifically, will docs still build correctly)?

Yep, it has no effect at least in my testing; the checks in the script run fine and the docs build is byte for byte identical to the original (aside from what looks to be a couple hundred bytes of non-deterministic state in a single cached pickle). Only the imports under if TYPE_CHECKING are changed which only affect the code when it is statically evaluated by the type checker.

However, when running mypy I realized I'd made a silly mistake above, as praw is of course not a name at the top-level of the package, but is the top-level package itself, and thus was not getting imported successfully. The way to fix this was simply doing an absolute import; I verified everything still worked in an environment without praw installed to make sure it was still picking up the local copy, and it did, including testing linting, docs, mypy and running a full suite of tests with my application using the dev version. I've corrected the OP above to that effect, and will submit a preliminary PR with this change for review following this comment.

As a sidenote, I'm not sure I understand why there's a hand-rolled manual script for this, instead of simply adding them to your existing pre-commit config to automatically install, run and manage them as actual pre-commit/pre-push hooks. On top of that, all the deps and command line strings are duplicated again as individual Github Action run steps, in addition to the black duplication in the pre-commit config, when everything could be single-sourced in the pre-commit config file and run in one line with the pre-commit Github action. If its simply a matter of no one getting the time to move it over, I'm happy to try to help there.

[LilSpazJoekp]

There is a difference between the pre_push script and GitHub actions. The GitHub actions only check and will fail if the codebase is not to standard. Feel free to open a PR though regarding the hooks and I'll take a look.

Thanks!

[CAM-Gerlach]

The GitHub actions only check and will fail if the codebase is not to standard.

Yeah, but that's exactly what running pre-commit via a Github action on CI will do, unless you run it via Pre-Commit.CI instead (which can automatically apply the fixes if configured to do so), while running pre-commit locally (manually or automatically) will actually make changes to the to-be-committed files and give the contributor the chance to review them (unstaged, so its easy for the contributor to see what it modified) before commiting.

I'll go ahead with that when I have the time, thanks. PR for this issue coming momentarily.

[CAM-Gerlach]

@LilSpazJoekp PR up; thanks for your help. Just to note, for the record so it gets linked here for reference, it looks like #1676 might have been the PR that broke this, based on git blame.