-
Notifications
You must be signed in to change notification settings - Fork 49
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
Exclude modules from the graph, according to wildcard expressions #44
Comments
Hi Iago, thanks for the feature request. A good way to go with this, I think, is to have a configuration option that allows modules to be ignored (currently you can only ignore relationships between modules, using the I think this is potentially a good idea, I can't give you a date by which I'd implement it though. I'd certainly consider a pull request for it. In the meantime you could implement this feature using a custom contract type. The approach would be to subclass
|
Thanks for quick answer, i will try your solution and i also will try to have some time to make a PR adding wildcard support. |
Great - let me know if you need any pointers. |
Can't make it work with a custom contract. Even if i just subclass [importlinter]
root_package = apps
contract_types =
custom_layer: docs.layers.CustomLayersContract class CustomLayersContract(LayersContract):
pass Results in: 'ListField' object is not iterable |
You've discovered a bug! I've been able to reproduce this and have added it as an issue here: #45 In the meantime, the workaround is just to redeclare the fields explicitly in your custom contract, i.e.
|
@seddonym @iagocanalejas this issue and the PR seem a little bit outdated 😄 I would like to extend this feature request to the independence type. Basically the ignore option should allow wildcards. If you like, I can give it a try building on top of the current PR. This would also help a project of mine where else we need to define a long list of ignores due the extensive usage of private submodules |
Hi @kasium - thanks for reaching out. I'd be happy to review a PR from you if you have capacity to move this forward - might be easier to start a dedicated PR though. |
@seddonym before I start a simple question: What kind of wildcards do we want to support?
I would go for the second one since both are invalid identifier names in python |
Simple wildcards I think. See https://docs.python.org/3/library/fnmatch.html. Have a look too at my comment on the other PR. |
I see that make sense. So for which fields to you see this feature? Personally, I only have demand to have it for ignores and there only on the left side. Implementation wise, I currently think of a special class WildcardModule or so. Only supporting it for ignores would make it more easy to add, because it would only affect the pop_imports method |
Let's stick to
It's probably simpler for it to be on both sides, so we have something like this?
I reckon probably the best direction is to adjust the Let me know if you want to chat further before getting stuck in. |
Thanks a lot for your support. I tried to get some more information and ideas, and here are my (hopefully) final suggestion: I guess that
So in short, instead of letting the user write down any combination, we generate it for him/her. This idea feels to be more elegant and doesn't require changes in What do you think about this? |
I think the simplest approach would be to allow wildcards in The contracts implementing
helpers.pop_imports then becomes:
I don't see why we can't support 'root' wildcards. The graph can support multiple root packages (they will have been passed in as How does that sound? |
Thanks for the suggestion. My worry about moving the "explosion" Also in |
Hmm, you make a good point. I don't think The challenge is that up until this point, Perhaps it would make more sense to introduce a new class similar to We could leave
Any better? |
Let me give it a try after the weekend. I like the idea |
Allow ignored regex expressions in layers contract.
As a example y don't want my
*.tests.*
modules to be processed.The text was updated successfully, but these errors were encountered: