kedro icon indicating copy to clipboard operation
kedro copied to clipboard

Replace Pipeline with pipeline across all tests

Open jmholzer opened this issue 2 years ago • 2 comments

Description

Currently, Pipeline is used to instantiate objects of this class across the testing code base. This should be updated to use the pipeline factory in kedro.pipeline.modular_pipeline.

Context

This comes from @AntonyMilneQB's comment on #1795 suggesting that these changes be made for the test files modified for that feature.

jmholzer avatar Sep 06 '22 16:09 jmholzer

This is my first issue and I'd like to be assigned

ziyad00 avatar Sep 17 '22 11:09 ziyad00

Excellent! So happy to have you onboard @ziyad00

jmholzer avatar Sep 18 '22 20:09 jmholzer

Is this issue still open? I'd be happy to pick up the work for it

adamfrly avatar Jan 09 '23 23:01 adamfrly

I believe it is indeed still open. @jmholzer?

antonymilne avatar Jan 10 '23 10:01 antonymilne

Hey @adamfrly, yes this issue is still open 🙂 Awesome! Would be glad to have your contribution.

jmholzer avatar Jan 10 '23 10:01 jmholzer

From what I recall I think all the remaining instances of Pipeline cannot be fixed just by find and replace (all the ones that could have already been) since there would be a clash of variable name and function name pipeline. Hence there would be a need to rename some variables here, not just do a naive find and replace. Does that sound right @jmholzer?

antonymilne avatar Jan 10 '23 10:01 antonymilne

@AntonyMilneQB yes! This is exactly right 🙂. As soon as we import pipeline there will be a good deal of shadow naming going on in many of the files, so variable names will have to be changed.

jmholzer avatar Jan 10 '23 11:01 jmholzer

Sounds great! I was thinking of something along the lines of from kedro.pipeline.modular_pipeline import pipeline as modular_pipeline to avoid the clash

adamfrly avatar Jan 10 '23 12:01 adamfrly