kedro-org / kedro-viz

Visualise your Kedro data and machine-learning pipelines and track your experiments.
https://demo.kedro.org
Apache License 2.0
682 stars 113 forks source link

Breakdown flowchart models into separate files #2144

Closed SajidAlamQB closed 3 weeks ago

SajidAlamQB commented 1 month ago

Description

Related to: https://github.com/kedro-org/kedro-viz/issues/1909

This PR refactors the flowchart.py module by breaking down its contents into separate, more maintainable files within the kedro_viz.models.

What was done:

Development notes

nodes.py: Contains node related classes such as GraphNode, TaskNode, DataNode, TranscodedDataNode, ParametersNode, and ModularPipelineNode.

node_metadata.py: Contains metadata classes for nodes, including GraphNodeMetadata, TaskNodeMetadata, DataNodeMetadata, TranscodedDataNodeMetadata, and ParametersNodeMetadata.

edges.py: Contains the GraphEdge class.

pipelines.py: Contains pipeline-related classes such as RegisteredPipeline and ModularPipelineNode.

tag.py: Contains the Tag class.

model_utils.py: Contains shared utilities and base classes like NamedEntity, GraphNodeType, ModularPipelineChild, and utility functions (_parse_filepath, _extract_wrapped_func).

QA notes

Checklist

rashidakanchwala commented 4 weeks ago

Hey @SajidAlamQB , thanks for the PR and for taking the time to walk me through your changes earlier. The new structure looks great and is quite easy to follow. I do have a few observations that took me a bit to fully understand. It seems there are foundational classes, or base classes, that other node classes (like TaskNode) inherit from, such as GraphNodeMetadata and NodeGraph. I was thinking it might make sense to introduce a base.py file to separate these foundational classes from their derived ones.

This approach could bring a few benefits:

  1. Separation of Concerns: Clearly distinguishing base classes from the other classes that inherit improves clarity around what’s foundational versus specialized.
  2. Readability and Maintainability: It would make it easier to locate and understand the main functionality provided by the base classes.

@rashidakanchwala Do you think this structure would work with our codebase? My knowledge with the back-end is somewhat limited, so please let me know if this approach wouldn’t be a good fit for our application.

Hey @Huong, thanks for sharing your thoughts! I agree that separating foundational classes can sometimes enhance readability, but for now, I think our current setup works well.

Since GraphNodes is in nodes.py along with its children classes and GraphNodeMetadata is in nodes_metadata.py with all NodeMetadata subclasses, this structure already keeps related classes together, which feels cohesive and reduces extra navigation.