Skip to content

Implement usethis tool import-linter #98

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

Closed
nathanjmcdougall opened this issue Oct 31, 2024 · 9 comments
Closed

Implement usethis tool import-linter #98

nathanjmcdougall opened this issue Oct 31, 2024 · 9 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@nathanjmcdougall
Copy link
Owner

Motivation
This is another super powerful (and underrated) linter which is configured for the project but would be worth supporting in usethis.

Summary of desired enhancement
We want a usethis tool import-linter interface.

The tricky part is deciding how to add contracts. Possibly there is some clever logic we can use to guess a good contract. Alternatively, we could provide a template and ask the user to populate it. We can point the user to the docs.

@nathanjmcdougall nathanjmcdougall self-assigned this Oct 31, 2024
@nathanjmcdougall nathanjmcdougall added the enhancement New feature or request label Oct 31, 2024
@nathanjmcdougall
Copy link
Owner Author

As a default, we could create an exhaustive, layer contract with sibling-relationships allowed between all modules, put into the same layer. We would ask the user to manually strengthen the contract.

At a later point, perhaps we can create a contract guesser which tries to do something more powerful.

@nathanjmcdougall
Copy link
Owner Author

There are a few bugs to fix first:
#183, #184.

Also, #185 insofar as it will involve supporting setup.cfg which is one way of configuring import-linter.

@nathanjmcdougall
Copy link
Owner Author

As far as config goes, something like this, as a draft:

# https://import-linter.readthedocs.io/en/stable/usage.html#configuration-file-location

def get_managed_files(self) -> list[Path]:
    return [Path(".importlinter")]

def get_managed_setup_cfg_keys(self) -> list[list[str]]:
    return [["importlinter"]]

def get_managed_pyproject_keys(self) -> list[list[str]]:
    return [["tool", "importlinter"]]

And then for the algorithm, we would go through src and find all subpackages. For each subpackage, we'd get the module names and set up a contract, similar to how it's done for usethis.

In our box_print message when adding the tool, we need to note that the command is lint-imports not import-linter.

@nathanjmcdougall
Copy link
Owner Author

We've got a function which provides a layered architecture for a given package, ready for use in import-linter config.

A few edge cases to handle:

  • What happens if there are no detected packages? Do we create one? Do we just display a message explaining how to setup and run the tool?
  • What do we do if there are no layered architectures associated with any modules (e.g. it's singleton; at least I think that's an example)?
  • Suppose the entire architecture is in the "excluded" list due to cycles. What do we do then?

Actually, all these edge cases are basically in the same category of "don't have any configuration to put down".
There might be a way to do placeholder configuration, but I'd need to experiment.

@nathanjmcdougall
Copy link
Owner Author

nathanjmcdougall commented Apr 4, 2025

Another edge case: import-linter assumes properly configured packages, i.e. not missing __init__.py files.
If we don't find any properly configured packages, it falls into the above case of "we don't have any config to put down" but in addition we should warn the user or take another action to try and help them cope, e.g. perhaps we have an "ensure init.py" method they can run. I think it depends on how we handle placeholder cases.

This is related to #128.

@nathanjmcdougall
Copy link
Owner Author

Ah, easy, via experiment it turns out that empty config works ok:

[tool.importlinter]
root_packages = [ "example" ]

[[tool.importlinter.contracts]]
name = "Modular Design"
type = "layers"
layers = []
containers = [ "example" ]
exhaustive = true

@nathanjmcdougall
Copy link
Owner Author

Actually, that doesn't necessarily solve the case where there is no package at all though, only an empty package.

I think warning about the need for __init__.py is a good idea in the usage instructions.

@nathanjmcdougall
Copy link
Owner Author

nathanjmcdougall commented Apr 6, 2025

If we can't find any packages, I reckon we just exit with an error, and messages explaining that a package is required. We could add a hint suggesting __init__.py files need to be present.

Alternatively, we could use the project name and add an empty contract. I think that's better but we should display a warning.

We'd also need to regularize the name so that it's a valid Python package name.

@nathanjmcdougall nathanjmcdougall linked a pull request Apr 7, 2025 that will close this issue
@nathanjmcdougall
Copy link
Owner Author

Closed by #528.

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

Successfully merging a pull request may close this issue.

1 participant