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

Ability to group modules together #61

Closed
tolomea opened this issue Aug 7, 2019 · 5 comments
Closed

Ability to group modules together #61

tolomea opened this issue Aug 7, 2019 · 5 comments
Assignees

Comments

@tolomea
Copy link

tolomea commented Aug 7, 2019

Currently the various contracts require one module per line.
It would be nice to be able to have multiple per line and to have that be semantically different from when they are on separate lines.
Specifically I'm thinking that where there are two on one line it would say nothing about the relationship between the two just that they have the same relationship to the others.
Formally where there are two on one line it would be equivalent to evaluating the rule twice, one time with each of them in that space.
With regard to one of my current projects this would let me write this:

[importlinter:contract:4]
name = Config can't import Runner et al
type = forbidden
source_modules =
    kedro.config
forbidden_modules =
    kedro.runner
    kedro.io
    kedro.pipeline

[importlinter:contract:5]
name = Runner et al can't import Config
type = forbidden
source_modules =
    kedro.runner
    kedro.io
    kedro.pipeline
forbidden_modules =
    kedro.config

as

[importlinter:contract:4]
name = Config and Runner et al are independent
type = independence
modules =
    kedro.config
    kedro.runner kedro.io kedro.pipeline
@seddonym
Copy link
Owner

seddonym commented Aug 7, 2019

Thanks for the suggestion - yes, I see what you mean. Particularly in this case, it would be much better if this could be expressed in terms of a single contract.

This concept could be thought of as the ability to group modules together. I'm going to rename the issue accordingly. I think it's worth doing at some point.

@seddonym seddonym changed the title Multiple modules per line Ability to group modules together Aug 7, 2019
@seddonym seddonym self-assigned this Aug 7, 2019
@uhbif19
Copy link

uhbif19 commented Feb 5, 2020

@seddonym

This concept could be thought of as the ability to group modules together

As far as I understand, this is possible only for independence contract. Am I correct?

@tolomea
Copy link
Author

tolomea commented Feb 6, 2020

@uhbif19
I don't know about implementation wise, but semantically it would work fine for the layer contract as well. So this:

[importlinter:contract:1]
name = Example
type = layers
layers =
    tom
    harry fred
    james

Would say that harry and james are in the same layer and have the same relationship to the other layers, but at least as far as this rule is concerned they are free to import each other.

That kinda gets at larger import linter things that are kicking around in the back of my mind. I feel a bit like instead of layers a generic DAG contract would be nice, this change would move layers a little closer to that. The problem with a generic DAG contract is how do you specify it in a way that is comprehensible.

@seddonym
Copy link
Owner

seddonym commented Feb 6, 2020

I agree with @tolomea. This would work for other contract types (but it's not currently implemented for any contract type).

There is a workaround, which is to locate both modules within a common subpackage.

For example, in the first example, you could move runner, io and pipeline into the same shared module (in this case I've called it core) and then have this contract:

[importlinter:contract:1]
name = Config and Runner et al are independent
type = independence
modules =
    kedro.config
    kedro.core

Personally, I would find it easier to understand the architectural intentions of a project that was structured in that way.

@seddonym
Copy link
Owner

seddonym commented Feb 3, 2023

I think this is covered by this issue now. #157

@seddonym seddonym closed this as completed Feb 3, 2023
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

3 participants