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

Fix: lambda python functions not rendering correctly on flowchart #851

Merged
merged 24 commits into from
May 20, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ac1bd25
update demo project to 0.18.1
tynandebold May 12, 2022
1bf863d
fix the bug
tynandebold May 16, 2022
74564b2
update release notes
tynandebold May 16, 2022
48b530a
Merge branch 'main' of https://github.com/kedro-org/kedro-viz into fi…
tynandebold May 16, 2022
4d8dec4
better implementation
tynandebold May 16, 2022
8c216a7
fix search highlighting
tynandebold May 16, 2022
acd397a
cleaner
tynandebold May 16, 2022
af2eba7
update releaes notes
tynandebold May 16, 2022
1f6eb76
Merge branch 'main' of https://github.com/kedro-org/kedro-viz into fi…
tynandebold May 16, 2022
f2923b8
update
tynandebold May 17, 2022
8692956
z-index search close icon
tynandebold May 17, 2022
54a18d5
added to readme
rashidakanchwala May 18, 2022
450a1af
update
tynandebold May 18, 2022
4a271ba
Merge branch 'fix/lambda-python-functions-on-flowchart' of https://gi…
tynandebold May 18, 2022
9df69c8
Merge branch 'main' of https://github.com/kedro-org/kedro-viz into fi…
tynandebold May 18, 2022
c152a64
undo readme changes
rashidakanchwala May 18, 2022
b3923fb
Merge branch 'fix/lambda-python-functions-on-flowchart' of github.com…
rashidakanchwala May 18, 2022
7b08e6f
undo readme changes
rashidakanchwala May 18, 2022
bebec6c
undo plotly readme changes
rashidakanchwala May 18, 2022
0bfe837
undo readme-2
rashidakanchwala May 18, 2022
45240cc
cleanup
tynandebold May 18, 2022
abbea45
Merge branch 'fix/lambda-python-functions-on-flowchart' of https://gi…
tynandebold May 18, 2022
1027bd9
Merge branch 'main' of https://github.com/kedro-org/kedro-viz into fi…
tynandebold May 18, 2022
7c25f75
Merge branch 'main' into fix/lambda-python-functions-on-flowchart
tynandebold May 20, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Please follow the established format:

## Bug fixes and other changes

- Fix lambda and partial Python functions not rendering correctly on flowchart. (#851)
- Add tooltip label text to page-navigation links. (#846)

# Release 4.5.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ def create_pipeline(**kwargs) -> Pipeline:
outputs=["prm_shuttle_company_reviews", "prm_spine_table"],
name="combine_step",
),
node(
func=lambda x: x,
inputs='x',
outputs='y'
)
],
namespace="ingestion", # provide inputs
inputs={"reviews", "shuttles", "companies"}, # map inputs outside of namespace
Expand Down
8 changes: 2 additions & 6 deletions package/kedro_viz/models/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ def _strip_namespace(name: str) -> str:
return re.sub(pattern, "", name)


def _strip_tags(name: str) -> str:
return name.replace("<", "&lt;").replace(">", "&gt;")
Comment on lines -46 to -47
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the solve for this is to leave the <> in place in the backend and then change it to use the HTML entities only where we need it in the frontend.



def _parse_filepath(dataset_description: Dict[str, Any]) -> Optional[str]:
filepath = dataset_description.get("filepath") or dataset_description.get("path")
return str(filepath) if filepath else None
Expand Down Expand Up @@ -185,8 +181,8 @@ def create_task_node(cls, node: KedroNode) -> "TaskNode":
node_name = node._name or node._func_name
return TaskNode(
id=cls._hash(str(node)),
name=_pretty_name(_strip_tags(node_name)),
full_name=_strip_tags(node_name),
name=_pretty_name(node_name),
full_name=node_name,
tags=set(node.tags),
kedro_obj=node,
)
Expand Down
4 changes: 2 additions & 2 deletions package/tests/test_models/test_graph/test_graph_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,8 @@ def test_task_node_metadata_with_partial_func(self):
)
task_node = GraphNode.create_task_node(kedro_node)
task_node_metadata = TaskNodeMetadata(task_node=task_node)
assert task_node.name == "&lt;partial&gt;"
assert task_node.full_name == "&lt;partial&gt;"
assert task_node.name == "<partial>"
assert task_node.full_name == "<partial>"
assert not hasattr(task_node_metadata, "code")
assert not hasattr(task_node_metadata, "filepath")
assert task_node_metadata.parameters == {}
Expand Down
5 changes: 1 addition & 4 deletions src/components/metadata/metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,7 @@ const MetaData = ({
className="pipeline-metadata__icon"
icon={nodeTypeIcon}
/>
<h2
className="pipeline-metadata__title"
dangerouslySetInnerHTML={{ __html: metadata.name }}
/>
<h2 className="pipeline-metadata__title">{metadata.name}</h2>
</div>
<IconButton
container={React.Fragment}
Expand Down
13 changes: 11 additions & 2 deletions src/components/node-list/node-list-row.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { memo } from 'react';
import { connect } from 'react-redux';
import classnames from 'classnames';
import { changed } from '../../utils';
import { changed, replaceMatches } from '../../utils';
import NodeIcon from '../icons/node-icon';
import VisibleIcon from '../icons/visible';
import InvisibleIcon from '../icons/invisible';
Expand All @@ -11,6 +11,13 @@ import { getNodeActive } from '../../selectors/nodes';
// The exact fixed height of a row as measured by getBoundingClientRect()
export const nodeListRowHeight = 37;

// HTML tags to replace partial and curry Python functions so we
// don't get a conflict with using dangerouslySetInnerHTML
const stringsToReplace = {
'<lambda>': '&lt;lambda&gt;',
'<partial>': '&lt;partial&gt;',
};

/**
* Returns `true` if there are no props changes, therefore the last render can be reused.
* Performance: Checks only the minimal set of props known to change after first render.
Expand Down Expand Up @@ -124,7 +131,9 @@ const NodeListRow = memo(
'pipeline-nodelist__row__label--disabled': disabled,
}
)}
dangerouslySetInnerHTML={{ __html: label }}
dangerouslySetInnerHTML={{
__html: replaceMatches(label, stringsToReplace),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace the <> here so we can still use dangerouslySetInnerHTML without a problem, specifically when searching.

}}
/>
</TextButton>
{typeof count === 'number' && (
Expand Down
18 changes: 18 additions & 0 deletions src/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,21 @@ export const changed = (props, objectA, objectB) => {
objectA && objectB && props.some((prop) => objectA[prop] !== objectB[prop])
);
};

/**
* Replace any parts of a string that match the keys in the toReplace object
* @param {string} str The string to check
* @param {object} toReplace The object of strings to replace and their replacements
* @returns {string} The string with or without replaced values
*/
export const replaceMatches = (str, toReplace) => {
if (str?.length > 0) {
const regex = new RegExp(Object.keys(toReplace).join('|'), 'gi');

return str.replace(regex, (matched) => {
return toReplace[matched];
});
} else {
return str;
}
};
15 changes: 14 additions & 1 deletion src/utils/utils.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { arrayToObject, getUrl, unique } from './index';
import { arrayToObject, getUrl, unique, replaceMatches } from './index';

describe('utils', () => {
describe('arrayToObject', () => {
Expand Down Expand Up @@ -46,4 +46,17 @@ describe('utils', () => {
expect([1, 1, 2, 2, 3, 3, 1].filter(unique)).toEqual([1, 2, 3]);
});
});

describe('replaceMatches', () => {
const entitiesToReplace = {
'&lt;': '<',
'&gt;': '>',
};

it('replaces matched characters from a string', () => {
expect(replaceMatches('&lt;lambda&gt;', entitiesToReplace)).toEqual(
'<lambda>'
);
});
});
});