strict-imports toggle doesn't seem to work when run by pre-commit

[Goldziher]

Hi,

I tried configuring no-strict-imports i pyproject.toml, but running the tool via pre-commit ignored this completely. I concluded it probably doesn't read the pyproject.toml file inside pre-commit, so I insterad tried doing this in my pre-commit config:

  - repo: https://github.com/ariebovenberg/slotscheck
    rev: v0.14.1
    hooks:
      - id: slotscheck
        exclude: "test_*"
        args: [--no-strict-imports]

But the tool keeps complaining about imports. I think it simply doesn't pick the config.

[ariebovenberg]

I'll have a look 👀

[ariebovenberg]

@Goldziher I did some digging...I don't think the pyproject.toml is an issue, rather something else. You can confirm the pyproject.toml is loaded by making a typo in the keys. Slotscheck will print an informative message. If there is no message, you can be sure slotscheck is not reading it.

slotscheck.config.InvalidKeys: Invalid configuration key(s): 'strict-impots'.

What I think may be the issue instead is the fact that slotscheck never ignores failed imports of root modules (modules that don't have a parent). At the time this seemed a different category of error (it's the entire package that can't be imported, not a subcomponent), but I suppose it's more straightforward to also ignore these type of errors. Does your import error happen to occur on a root module? If not, could you share some more of your setup?

[Goldziher]

@Goldziher I did some digging...I don't think the pyproject.toml is an issue, rather something else. You can confirm the pyproject.toml is loaded by making a typo in the keys. Slotscheck will print an informative message. If there is no message, you can be sure slotscheck is not reading it.

slotscheck.config.InvalidKeys: Invalid configuration key(s): 'strict-impots'.

What I think may be the issue instead is the fact that slotscheck never ignores failed imports of root modules (modules that don't have a parent). At the time this seemed a different category of error (it's the entire package that can't be imported, not a subcomponent), but I suppose it's more straightforward to also ignore these type of errors. Does your import error happen to occur on a root module? If not, could you share some more of your setup?

Hi, they happen on all dependencies.

I will add a branch later and send you a link here so you can see.

[Goldziher]

Ok, here is a branch in our repo: https://github.com/starlite-api/starlite/tree/demonstrate-slots-check-issue

Simply clone it and execute pre-commit install && pre-commit run slotscheck --all-files.

This will fail completely, so to "fix" it you will need to update the .pre-commit-config.yaml to look like it looks in main, namely this:

  - repo: https://github.com/ariebovenberg/slotscheck
    rev: v0.14.1
    hooks:
      - id: slotscheck
        exclude: "test_*"
        additional_dependencies:
          [
            aiomcache,
            brotli,
            cryptography,
            httpx,
            jinja2,
            mako,
            orjson,
            piccolo,
            picologging,
            pydantic,
            pydantic_factories,
            pydantic_openapi_schema,
            pytest,
            pyyaml,
            redis,
            sqlalchemy,
            starlette,
            starlite_multipart,
            structlog,
            tortoise_orm,
          ]

Which includes all the above dependencies, which are supposed to be ignored.

[ariebovenberg]

@Goldziher thanks for taking the time to share your setup. The problem is indeed that not all import errors are ignored. In short, slotscheck runs a few basic checks on all files paths given, which may cause ungraceful import errors. Since pre-commit passes all files separately, it occurs only in that case.

Solution is of course to gracefully handle these as well (if strict-imports is disabled). I'll have a fix out soon for that.

[ariebovenberg]

@Goldziher alright, the pending PR should fix this. You can try it yourself by using the tag v0.16b2 in pre-commit. New release should be out soon.

[ariebovenberg]

@Goldziher this issue got auto-closed with the PR. Feel free to re-open if it doesn't solve your issue.

edit: the fix is now out in version 0.16.0 on PyPI

[Goldziher]

Cheers