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.

Sign up for GitHub

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

Add preview to datasets as specified in the Kedro catalog under metadata #1374

Merged
merged 16 commits into from
Jun 8, 2023
Merged

Add preview to datasets as specified in the Kedro catalog under metadata #1374

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e2086a3
done with failing lint and tests which will be fixed once other PR is…
rashidakanchwala May 30, 2023
859c359
changed to preview_args
rashidakanchwala Jun 1, 2023
1257a66
fixes based on reviews
rashidakanchwala Jun 5, 2023
bee0771
Update metadata-modal.js
rashidakanchwala Jun 6, 2023
71ff583
added viz_metdata to property
rashidakanchwala Jun 6, 2023
dd168ae
Merge branch 'preview-datasets-2' of https://github.com/kedro-org/ked…
rashidakanchwala Jun 6, 2023
faac571
remove default to None
rashidakanchwala Jun 6, 2023
9346fb8
changes based on review
rashidakanchwala Jun 6, 2023
0c98271
review fixes
rashidakanchwala Jun 6, 2023
a241bf5
Merge branch 'main' into preview-datasets-2
rashidakanchwala Jun 7, 2023
e604031
fix lint and tests
rashidakanchwala Jun 7, 2023
d5ed369
fix lint
rashidakanchwala Jun 7, 2023
90ea841
fix space
rashidakanchwala Jun 7, 2023
4d85ddb
fix lint
rashidakanchwala Jun 7, 2023
4ee8367
add release notes and test
rashidakanchwala Jun 8, 2023
6cbd29f
Merge branch 'main' into preview-datasets-2
rashidakanchwala Jun 8, 2023
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
22 changes: 5 additions & 17 deletions demo-project/conf/base/catalog_01_raw.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,26 @@ companies:
metadata:
kedro-viz:
layer: raw
preview: 5
preview_args:
merelcht marked this conversation as resolved.
Show resolved Hide resolved

reviews:
type: pandas.CSVDataSet
filepath: ${base_location}/01_raw/reviews.csv
metadata:
kedro-viz:
layer: raw
preview: 10
preview_args:
nrows: 10

shuttles:
type: pandas.ExcelDataSet
filepath: ${base_location}/01_raw/shuttles.xlsx
metadata:
kedro-viz:
layer: raw
preview: 15
preview_args:
nrows: 15




# companies:
# type: pandas.CSVDataSet
# filepath: ${base_location}/01_raw/companies.csv
# layer: raw

# reviews:
# type: pandas.CSVDataSet
# filepath: ${base_location}/01_raw/reviews.csv
# layer: raw

# shuttles:
# type: pandas.ExcelDataSet
# filepath: ${base_location}/01_raw/shuttles.xlsx
# layer: raw
22 changes: 18 additions & 4 deletions package/kedro_viz/models/flowchart.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,14 +486,28 @@ def is_tracking_node(self):
"""Checks if the current node is a tracking data node"""
return self.is_json_node() or self.is_metric_node()

def get_viz_metadata(self):
"""Gets the kedro-viz metadata config specified in the catalog"""
metadata = getattr(self.kedro_obj, "metadata", None)
if not metadata:
return None
try:
viz_metadata = metadata["kedro-viz"]
except (AttributeError, KeyError):
return None
return viz_metadata
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be replaced with the getattr method since it's not a nested key?

Copy link
Contributor

Choose a reason for hiding this comment

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

additional comment: Would it be better to just set this as a property viz_metadata instead of a get method?

Copy link
Contributor Author

@rashidakanchwala rashidakanchwala Jun 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

i did try this but I got some weird errors, so changed it to get_ . but i agree, it's better as a property. I will ping u again, if u have a couple of min to help me with the errors that would be appreciated. let me try doing it again.

Update - it works now, i will push :)

Copy link
Contributor Author

@rashidakanchwala rashidakanchwala Jun 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Antony suggested doing it this way based on EAFP

try:
       viz_metadata = metadata["kedro-viz"]
 except (AttributeError, KeyError):


def is_preview_node(self):
"""Checks if the current node has a preview"""
metadata = getattr(self.kedro_obj, "metadata", {}) or {}
return bool(metadata.get("kedro-viz", {}).get("preview"))
try:
is_preview = bool(self.get_viz_metadata()["preview_args"])
except (AttributeError, KeyError):
return False
return is_preview

def get_preview_nrows(self):
Copy link
Member

Choose a reason for hiding this comment

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

This gets all preview_args right? So I think it makes more sense to name the function get_preview_args

"""Gets the number of rows for the preview dataset"""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""Gets the number of rows for the preview dataset"""
"""Gets the preview arguments for a dataset"""

return int(self.kedro_obj.metadata.get("kedro-viz", {}).get("preview", 0))
return self.get_viz_metadata()["preview_args"]


@dataclass
Expand Down Expand Up @@ -600,7 +614,7 @@ def __post_init__(self, data_node: DataNode):
self.tracking_data = dataset.load()
elif data_node.is_preview_node():
try:
self.preview = dataset._preview(data_node.get_preview_nrows())
self.preview = dataset._preview(**data_node.get_preview_nrows())
except Exception as exc: # pylint: disable=broad-except # pragma: no cover
logger.warning(
"'%s' could not be previewed. Full exception: %s: %s",
Expand Down
2 changes: 1 addition & 1 deletion src/components/metadata-modal/metadata-modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const MetadataModal = ({ metadata, onToggle, visible }) => {
</div>
{hasPreview && (
<div className="pipeline-metadata-modal__preview-text">
Previewing first {metadata.preview.data.length} rows only
Previewing first {metadata.preview.data.length} rows
</div>
)}
</div>
Expand Down