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 tach check print more information about layer violations #585

Open
crockeo opened this issue Feb 1, 2025 · 1 comment
Open

make tach check print more information about layer violations #585

crockeo opened this issue Feb 1, 2025 · 1 comment

Comments

@crockeo
Copy link
Contributor

crockeo commented Feb 1, 2025

CodeDiagnostic only includes information about the tach modules where an issue occurs, not the Python module where they occur. This means tach check can't include all of the information which would allow a developer to identify a problem import without also opening the file in which it occurs.

As an anonymized example for a LayerCheckResult:

❌ path/to/python/file.py[L3]: Cannot import 'path.to.module'. Layer 'infrastructure' ('path.to.python') is lower than layer 'product' ('path.to.module').

I would then have to open path/to/python/file.py and see that the import in question is maybe something like:

from path.to.module.enums import SomeEnum

I think it would be useful for tach check to include information about both:

  • The Python module name of the file which contains the bad import (although this is easily inferred if you assume PYTHONPATH=.).
  • The Python module name of the imported module.

I'm happy to take a whack at it if y'all are ok with including this behavior.

@emdoyle
Copy link
Member

emdoyle commented Feb 1, 2025

I agree, the diagnostic should include the actual import module path! The intended behavior IIRC is for that first path (path.to.module in your example error) to be the full import path (e.g. path.to.module.enums.SomeEnum).

The Python module name of the file is less of an obvious need to me, since in general Tach's diagnostics try to reference module paths which appear in the config, and the filepath is clickable to get directly to the referenced line.

We'd definitely appreciate a PR here! I'm in the middle of a refactor (#586) but I should be able to easily transplant the fix -- might be another day or two before this merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants