kedro-viz icon indicating copy to clipboard operation
kedro-viz copied to clipboard

Refactor kedro_viz.services.modular_pipelines.py and kedro_viz.data_access.managers.py (partially)

Open jmholzer opened this issue 2 years ago • 0 comments

Description

In PR #1119, the code in these files was found to cause a rendering error. I found it difficult to work with the code in these files while attempting to fix this.

The specific targets in this issue are the functions / methods:

  • kedro_viz.services.modular_pipelines.expand_tree
  • kedro_viz.data_access.managers.DataAccessManager.create_modular_pipelines_tree_for_registered_pipeline
  • kedro_viz.data_access.managers.DataAccessManager.add_pipeline

Currently, the logic of these functions and methods is difficult to follow, and their output is not directly coverable by automated tests. This makes it easy to introduce a breaking change that can only be found through manual testing.

These functions have many responsibilities and, in the case of kedro_viz.services.modular_pipelines.expand_tree, a complicated execution path. The implementations of these responsibilities should be split across several new functions, each with dedicated doc strings and type annotations. This would make the logic easier to understand and modify.

Context

This will have two positive outcomes:

  1. It will be easier to investigate and fix #1123.
  2. It will simplify future development work.

Possible Implementation

  • Distributing the code across multiple well-documented functions / methods would be an excellent first step.
  • Eliminate (similar) repeated code.

Checklist

  • [x] Include labels so that we can categorise your feature request

jmholzer avatar Oct 05 '22 19:10 jmholzer