diff --git a/package/kedro_viz/services/layers.py b/package/kedro_viz/services/layers.py index 7cba369aa1..b7a46dd789 100644 --- a/package/kedro_viz/services/layers.py +++ b/package/kedro_viz/services/layers.py @@ -106,6 +106,11 @@ def find_child_layers(node_id: str) -> Set[str]: if layer not in layer_dependencies: layer_dependencies[layer] = set() + # Sort `layer_dependencies` keys for consistent ordering of layers with the same dependencies + layer_dependencies = defaultdict( + set, {k: layer_dependencies[k] for k in sorted(layer_dependencies)} + ) + # Use graphlib.TopologicalSorter to sort the layer dependencies. try: sorter = TopologicalSorter(layer_dependencies) diff --git a/package/tests/test_services/test_layers.py b/package/tests/test_services/test_layers.py index c949a9f98b..358cf8f505 100644 --- a/package/tests/test_services/test_layers.py +++ b/package/tests/test_services/test_layers.py @@ -154,6 +154,32 @@ {"node_1": {}, "node_2": {}}, ["a", "b"], ), + ( + # Case where if two layers e.g. `int` and `primary` layers share the same dependencies, they get sorted alphabetically. + """ + node_1(layer=raw) -> node_3(layer=int) + node_2(layer=raw) -> node_4(layer=primary) + node_3(layer=int) -> node_5(layer=feature) + node_4(layer=primary) -> node_6(layer=feature) + """, + { + "node_1": {"id": "node_1", "layer": "raw"}, + "node_2": {"id": "node_2", "layer": "raw"}, + "node_3": {"id": "node_3", "layer": "int"}, + "node_4": {"id": "node_4", "layer": "primary"}, + "node_5": {"id": "node_5", "layer": "feature"}, + "node_6": {"id": "node_6", "layer": "feature"}, + }, + { + "node_1": {"node_3"}, + "node_2": {"node_4"}, + "node_3": {"node_5"}, + "node_4": {"node_6"}, + "node_5": set(), + "node_6": set(), + }, + ["raw", "int", "primary", "feature"], + ), ], ) def test_sort_layers(graph_schema, nodes, node_dependencies, expected): @@ -170,7 +196,7 @@ def test_sort_layers(graph_schema, nodes, node_dependencies, expected): for node_id, node_dict in nodes.items() } sorted_layers = sort_layers(nodes, node_dependencies) - assert sorted(sorted_layers) == sorted(expected), graph_schema + assert sorted_layers == expected, graph_schema def test_sort_layers_should_return_empty_list_on_cyclic_layers(mocker): diff --git a/src/actions/graph.test.js b/src/actions/graph.test.js index 1ff8df3436..991eb5ad66 100644 --- a/src/actions/graph.test.js +++ b/src/actions/graph.test.js @@ -1,25 +1,37 @@ import { createStore } from 'redux'; import reducer from '../reducers'; -import { mockState } from '../utils/state.mock'; import { calculateGraph, updateGraph } from './graph'; import { getGraphInput } from '../selectors/layout'; +import { prepareState } from '../utils/state.mock'; +import spaceflights from '../utils/data/spaceflights.mock.json'; +import spaceflightsReordered from '../utils/data/spaceflights_reordered.mock.json'; +import { toggleModularPipelinesExpanded } from '../actions/modular-pipelines'; describe('graph actions', () => { + const getMockState = (data) => + prepareState({ + data, + beforeLayoutActions: [ + () => + toggleModularPipelinesExpanded(['data_science', 'data_processing']), + ], + }); + describe('calculateGraph', () => { it('returns updateGraph action if input is falsey', () => { expect(calculateGraph(null)).toEqual(updateGraph(null)); }); it('sets loading to true immediately', () => { - const store = createStore(reducer, mockState.spaceflights); + const store = createStore(reducer, getMockState(spaceflights)); expect(store.getState().loading.graph).not.toBe(true); - calculateGraph(getGraphInput(mockState.spaceflights))(store.dispatch); + calculateGraph(getGraphInput(getMockState(spaceflights)))(store.dispatch); expect(store.getState().loading.graph).toBe(true); }); it('sets loading to false and graph visibility to true after finishing calculation', () => { - const store = createStore(reducer, mockState.spaceflights); - return calculateGraph(getGraphInput(mockState.spaceflights))( + const store = createStore(reducer, getMockState(spaceflights)); + return calculateGraph(getGraphInput(getMockState(spaceflights)))( store.dispatch ).then(() => { const state = store.getState(); @@ -29,19 +41,51 @@ describe('graph actions', () => { }); it('calculates a graph', () => { - const state = Object.assign({}, mockState.spaceflights); - delete state.graph; - const store = createStore(reducer, state); + const initialState = { ...getMockState(spaceflights), graph: {} }; + const store = createStore(reducer, initialState); expect(store.getState().graph).toEqual({}); - return calculateGraph(getGraphInput(state))(store.dispatch).then(() => { - expect(store.getState().graph).toEqual( - expect.objectContaining({ - nodes: expect.any(Array), - edges: expect.any(Array), - size: expect.any(Object), - }) - ); - }); + return calculateGraph(getGraphInput(initialState))(store.dispatch).then( + () => { + expect(store.getState().graph).toEqual( + expect.objectContaining({ + nodes: expect.any(Array), + edges: expect.any(Array), + size: expect.any(Object), + }) + ); + } + ); + }); + + it('compares deterministic flowchart of two differently ordered same projects', () => { + const store1 = createStore(reducer, getMockState(spaceflights)); + const store2 = createStore(reducer, getMockState(spaceflightsReordered)); + + return calculateGraph(getGraphInput(getMockState(spaceflights)))( + store1.dispatch + ) + .then(() => + calculateGraph(getGraphInput(getMockState(spaceflightsReordered)))( + store2.dispatch + ) + ) + .then(() => { + // Get node coordinates for both graphs + const graph1Coords = store1.getState().graph.nodes.map((node) => ({ + id: node.id, + x: node.x, + y: node.y, + })); + + const graph2Coords = store2.getState().graph.nodes.map((node) => ({ + id: node.id, + x: node.x, + y: node.y, + })); + + // Verify coordinates consistency between both graphs + expect(graph1Coords).toEqual(expect.arrayContaining(graph2Coords)); + }); }); }); }); diff --git a/src/selectors/sliced-pipeline.test.js b/src/selectors/sliced-pipeline.test.js index 2a76049b2b..632eb63a9f 100644 --- a/src/selectors/sliced-pipeline.test.js +++ b/src/selectors/sliced-pipeline.test.js @@ -12,13 +12,13 @@ describe('Selectors', () => { const expected = [ '23c94afb', '47b81aa6', + '90ebe5f3', 'daf35ba0', 'c09084f2', '0abef172', 'e5a9ec27', 'b7bb7198', 'f192326a', - '90ebe5f3', 'data_processing', ]; const newState = reducer(mockState.spaceflights, { diff --git a/src/store/normalize-data.js b/src/store/normalize-data.js index 15aaacaa94..b38d4a2166 100644 --- a/src/store/normalize-data.js +++ b/src/store/normalize-data.js @@ -250,6 +250,15 @@ const getNodeTypesFromUrl = (state, typeQueryParams) => { return state; }; +/** + * Sort the edges, nodes in the state object to ensure deterministic graph layout + * @param {Object} state The state object to sort + */ +const sortNodesEdges = (state) => { + state.edge?.ids?.sort((a, b) => a.localeCompare(b)); + state.node?.ids?.sort((a, b) => a.localeCompare(b)); +}; + /** * Updates the state with filters from the URL. * @param {Object} state - State object @@ -331,6 +340,7 @@ const normalizeData = (data, expandAllPipelines) => { data.layers.forEach(addLayer(state)); } + sortNodesEdges(state); const updatedState = updateStateWithFilters(state, data.tags); return updatedState; }; diff --git a/src/store/normalize-data.test.js b/src/store/normalize-data.test.js index fbbc5dddd7..b02e7b95b4 100644 --- a/src/store/normalize-data.test.js +++ b/src/store/normalize-data.test.js @@ -1,5 +1,6 @@ import normalizeData, { createInitialPipelineState } from './normalize-data'; import spaceflights from '../utils/data/spaceflights.mock.json'; +import spaceflightsReordered from '../utils/data/spaceflights_reordered.mock.json'; const initialState = createInitialPipelineState(); @@ -90,4 +91,20 @@ describe('normalizeData', () => { expect(node).toHaveProperty('name'); }); }); + + it('should have identical nodes and edges, in the same order, regardless of the different ordering from the api', () => { + // Normalize both datasets + const initialState = normalizeData(spaceflights, true); + const reorderedState = normalizeData(spaceflightsReordered, true); + + // Compare nodes and edges by converting to JSON for deep equality + // Directly compare specific properties of nodes and edges, ensuring order and content + expect(initialState.node.ids).toEqual(reorderedState.node.ids); + expect(initialState.node.name).toEqual(reorderedState.node.name); + expect(initialState.node.type).toEqual(reorderedState.node.type); + + expect(initialState.edge.ids).toEqual(reorderedState.edge.ids); + expect(initialState.edge.sources).toEqual(reorderedState.edge.sources); + expect(initialState.edge.targets).toEqual(reorderedState.edge.targets); + }); }); diff --git a/src/utils/data/spaceflights_reordered.mock.json b/src/utils/data/spaceflights_reordered.mock.json new file mode 100644 index 0000000000..3dea125c60 --- /dev/null +++ b/src/utils/data/spaceflights_reordered.mock.json @@ -0,0 +1,311 @@ +{ + "nodes": [ + { + "id": "f192326a", + "name": "data_processing.shuttles", + "tags": ["preprocessing"], + "pipelines": ["dp", "__default__"], + "type": "data", + "modular_pipelines": ["data_processing"], + "layer": "raw", + "dataset_type": "pandas.excel_dataset.ExcelDataset" + }, + { + "id": "b7bb7198", + "name": "preprocess_shuttles_node", + "tags": ["preprocessing"], + "pipelines": ["dp", "__default__"], + "type": "task", + "modular_pipelines": ["data_processing"], + "parameters": {} + }, + { + "id": "e5a9ec27", + "name": "data_processing.preprocessed_shuttles", + "tags": ["features", "preprocessing"], + "pipelines": ["dp", "__default__"], + "type": "data", + "modular_pipelines": ["data_processing"], + "layer": "intermediate", + "dataset_type": "pandas.csv_dataset.CSVDataset" + }, + { + "id": "0abef172", + "name": "data_processing.companies", + "tags": ["preprocessing"], + "pipelines": ["dp", "__default__"], + "type": "data", + "modular_pipelines": ["data_processing"], + "layer": "raw", + "dataset_type": "pandas.csv_dataset.CSVDataset" + }, + { + "id": "daf35ba0", + "name": "data_processing.preprocessed_companies", + "tags": ["features", "preprocessing"], + "pipelines": ["dp", "__default__"], + "type": "data", + "modular_pipelines": ["data_processing"], + "layer": "intermediate", + "dataset_type": "pandas.csv_dataset.CSVDataset" + }, + { + "id": "c09084f2", + "name": "preprocess_companies_node", + "tags": ["preprocessing"], + "pipelines": ["dp", "__default__"], + "type": "task", + "modular_pipelines": ["data_processing"], + "parameters": {} + }, + { + "id": "47b81aa6", + "name": "create_model_input_table_node", + "tags": ["features"], + "pipelines": ["dp", "__default__"], + "type": "task", + "modular_pipelines": ["data_processing"], + "parameters": {} + }, + { + "id": "90ebe5f3", + "name": "data_processing.reviews", + "tags": ["features"], + "pipelines": ["dp", "__default__"], + "type": "data", + "modular_pipelines": ["data_processing"], + "layer": "raw", + "dataset_type": "pandas.csv_dataset.CSVDataset" + }, + { + "id": "65d0d789", + "name": "split_data_node", + "tags": ["split"], + "pipelines": ["__default__", "ds"], + "type": "task", + "modular_pipelines": ["data_science"], + "parameters": { + "test_size": 0.2, + "random_state": 3, + "features": [ + "engines", + "passenger_capacity", + "crew", + "d_check_complete", + "moon_clearance_complete", + "iata_approved", + "company_rating", + "review_scores_rating" + ] + } + }, + { + "id": "f1f1425b", + "name": "parameters", + "tags": ["split"], + "pipelines": ["__default__", "ds"], + "type": "parameters", + "modular_pipelines": [], + "layer": null, + "dataset_type": null + }, + { + "id": "23c94afb", + "name": "model_input_table", + "tags": ["features", "split"], + "pipelines": ["dp", "__default__", "ds"], + "type": "data", + "modular_pipelines": [], + "layer": "primary", + "dataset_type": "pandas.csv_dataset.CSVDataset" + }, + { + "id": "172a0602", + "name": "data_science.X_train", + "tags": ["split", "train"], + "pipelines": ["__default__", "ds"], + "type": "data", + "modular_pipelines": ["data_science"], + "layer": null, + "dataset_type": null + }, + { + "id": "9c2a8a5e", + "name": "data_science.X_test", + "tags": ["split"], + "pipelines": ["__default__", "ds"], + "type": "data", + "modular_pipelines": ["data_science"], + "layer": null, + "dataset_type": null + }, + { + "id": "e5cee9e2", + "name": "data_science.y_train", + "tags": ["split", "train"], + "pipelines": ["__default__", "ds"], + "type": "data", + "modular_pipelines": ["data_science"], + "layer": null, + "dataset_type": null + }, + { + "id": "ecc63a8c", + "name": "data_science.y_test", + "tags": ["split"], + "pipelines": ["__default__", "ds"], + "type": "data", + "modular_pipelines": ["data_science"], + "layer": null, + "dataset_type": null + }, + { + "id": "90f15f9d", + "name": "train_model_node", + "tags": ["train"], + "pipelines": ["__default__", "ds"], + "type": "task", + "modular_pipelines": ["data_science"], + "parameters": {} + }, + { + "id": "04424659", + "name": "data_science.regressor", + "tags": ["train"], + "pipelines": ["__default__", "ds"], + "type": "data", + "modular_pipelines": ["data_science"], + "layer": "models", + "dataset_type": "pickle.pickle_dataset.PickleDataset" + }, + { + "id": "f5e8d7df", + "name": "evaluate_model_node", + "tags": [], + "pipelines": ["__default__", "ds"], + "type": "task", + "modular_pipelines": ["data_science"], + "parameters": {} + }, + { + "id": "966b9734", + "name": "metrics", + "tags": [], + "pipelines": ["__default__", "ds"], + "type": "data", + "modular_pipelines": [], + "layer": null, + "dataset_type": "tracking.metrics_dataset.MetricsDataset" + }, + { + "id": "data_processing", + "name": "data_processing", + "tags": [], + "pipelines": ["dp", "__default__"], + "type": "modularPipeline", + "modular_pipelines": null, + "layer": null, + "dataset_type": null + }, + { + "id": "data_science", + "name": "data_science", + "tags": [], + "pipelines": ["__default__", "ds"], + "type": "modularPipeline", + "modular_pipelines": null, + "layer": null, + "dataset_type": null + } + ], + "edges": [ + { "source": "23c94afb", "target": "65d0d789" }, + { "source": "04424659", "target": "f5e8d7df" }, + { "source": "90ebe5f3", "target": "47b81aa6" }, + { "source": "23c94afb", "target": "data_science" }, + { "source": "daf35ba0", "target": "47b81aa6" }, + { "source": "data_science", "target": "966b9734" }, + { "source": "47b81aa6", "target": "23c94afb" }, + { "source": "e5cee9e2", "target": "90f15f9d" }, + { "source": "f192326a", "target": "b7bb7198" }, + { "source": "65d0d789", "target": "172a0602" }, + { "source": "f1f1425b", "target": "65d0d789" }, + { "source": "90ebe5f3", "target": "data_processing" }, + { "source": "f5e8d7df", "target": "966b9734" }, + { "source": "e5a9ec27", "target": "47b81aa6" }, + { "source": "65d0d789", "target": "9c2a8a5e" }, + { "source": "data_processing", "target": "23c94afb" }, + { "source": "f1f1425b", "target": "data_science" }, + { "source": "f192326a", "target": "data_processing" }, + { "source": "172a0602", "target": "90f15f9d" }, + { "source": "9c2a8a5e", "target": "f5e8d7df" }, + { "source": "ecc63a8c", "target": "f5e8d7df" }, + { "source": "b7bb7198", "target": "e5a9ec27" }, + { "source": "90f15f9d", "target": "04424659" }, + { "source": "65d0d789", "target": "e5cee9e2" }, + { "source": "0abef172", "target": "data_processing" }, + { "source": "65d0d789", "target": "ecc63a8c" }, + { "source": "0abef172", "target": "c09084f2" }, + { "source": "c09084f2", "target": "daf35ba0" } + ], + "layers": ["raw", "intermediate", "primary", "models"], + "tags": [ + { "id": "features", "name": "features" }, + { "id": "preprocessing", "name": "preprocessing" }, + { "id": "split", "name": "split" }, + { "id": "train", "name": "train" } + ], + "pipelines": [ + { "id": "__default__", "name": "__default__" }, + { "id": "dp", "name": "dp" }, + { "id": "ds", "name": "ds" } + ], + "modular_pipelines": { + "__root__": { + "id": "__root__", + "name": "__root__", + "inputs": [], + "outputs": [], + "children": [ + { "id": "data_processing", "type": "modularPipeline" }, + { "id": "23c94afb", "type": "data" }, + { "id": "966b9734", "type": "data" }, + { "id": "f1f1425b", "type": "parameters" }, + { "id": "data_science", "type": "modularPipeline" } + ] + }, + "data_processing": { + "id": "data_processing", + "name": "data_processing", + "inputs": ["90ebe5f3", "0abef172", "f192326a"], + "outputs": ["23c94afb"], + "children": [ + { "id": "e5a9ec27", "type": "data" }, + { "id": "f192326a", "type": "data" }, + { "id": "90ebe5f3", "type": "data" }, + { "id": "daf35ba0", "type": "data" }, + { "id": "47b81aa6", "type": "task" }, + { "id": "0abef172", "type": "data" }, + { "id": "c09084f2", "type": "task" }, + { "id": "b7bb7198", "type": "task" } + ] + }, + "data_science": { + "id": "data_science", + "name": "data_science", + "inputs": ["f1f1425b", "23c94afb"], + "outputs": ["966b9734"], + "children": [ + { "id": "ecc63a8c", "type": "data" }, + { "id": "172a0602", "type": "data" }, + { "id": "04424659", "type": "data" }, + { "id": "e5cee9e2", "type": "data" }, + { "id": "9c2a8a5e", "type": "data" }, + { "id": "f5e8d7df", "type": "task" }, + { "id": "65d0d789", "type": "task" }, + { "id": "90f15f9d", "type": "task" } + ] + } + }, + "selected_pipeline": "__default__" +} diff --git a/src/utils/state.mock.js b/src/utils/state.mock.js index 6b5d3268ac..e2141406bd 100644 --- a/src/utils/state.mock.js +++ b/src/utils/state.mock.js @@ -4,6 +4,7 @@ import { mount, shallow } from 'enzyme'; import configureStore from '../store'; import getInitialState from '../store/initial-state'; import spaceflights from './data/spaceflights.mock.json'; +import spaceflightsReordered from './data/spaceflights_reordered.mock.json'; import demo from './data/demo.mock.json'; import reducer from '../reducers'; import { getGraphInput } from '../selectors/layout'; @@ -53,6 +54,7 @@ export const mockState = { json: prepareState({ data: 'json' }), demo: prepareState({ data: demo }), spaceflights: prepareState({ data: spaceflights }), + spaceflightsReordered: prepareState({ data: spaceflightsReordered }), }; /**