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

Jump to bottom

Fix MemoryDataSet not displaying on metadata panel #1113

Merged
merged 12 commits into from
Oct 3, 2022
Merged

Fix MemoryDataSet not displaying on metadata panel #1113

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
823eed7
fixed memorydataset on backend
rashidakanchwala Oct 3, 2022
ef31de9
removed if memorydataset
rashidakanchwala Oct 3, 2022
f30a6b9
remove additions to flowchart.py
rashidakanchwala Oct 3, 2022
26ce669
remove additions to flowchart.py
rashidakanchwala Oct 3, 2022
5aaccb6
fix lint
rashidakanchwala Oct 3, 2022
85a2085
added to exception
rashidakanchwala Oct 3, 2022
909fced
fix lint, and add test, also fixes based on reviews
rashidakanchwala Oct 3, 2022
e998d0e
remove pragma:no cover
rashidakanchwala Oct 3, 2022
05fcacc
add release notes
rashidakanchwala Oct 3, 2022
4253cf3
add release notes
rashidakanchwala Oct 3, 2022
5d5177f
add release notes
rashidakanchwala Oct 3, 2022
13a65e7
Merge branch 'main' into fix-memory-dataset
rashidakanchwala Oct 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions demo-project/conf/base/catalog_05_model_input.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@ model_input_table:
type: pandas.ParquetDataSet
filepath: ${base_location}/05_model_input/model_input_table.pq
layer: model_input

X_train:
type: MemoryDataSet
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
X_train:
type: MemoryDataSet

I guess we can revert it?

2 changes: 1 addition & 1 deletion package/kedro_viz/api/rest/responses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class Config:


class DataNodeMetadataAPIResponse(BaseAPIResponse):
filepath: str
filepath: Optional[str]
type: str
plot: Optional[Dict]
image: Optional[str]
Expand Down
6 changes: 4 additions & 2 deletions package/kedro_viz/data_access/repositories/catalog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
centralise access to Kedro data catalog."""
# pylint: disable=missing-class-docstring,missing-function-docstring,protected-access
from typing import Dict, Optional
from joblib import Memory
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
from joblib import Memory


from kedro.io import AbstractDataSet, DataCatalog, DataSetNotFoundError
from kedro.io import AbstractDataSet, DataCatalog, DataSetNotFoundError, MemoryDataSet

from kedro_viz.constants import KEDRO_VERSION

Expand Down Expand Up @@ -55,8 +56,9 @@ def get_dataset(self, dataset_name: str) -> Optional[AbstractDataSet]:
dataset_obj = self._catalog._get_dataset(dataset_name, suggest=False)
else: # pragma: no cover
dataset_obj = self._catalog._get_dataset(dataset_name)
# if dataset has no catalog entry, it is a MemoryDataSet
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I think this is bordering on a redundant comment (the code seems really self-explanatory), but I suppose one could argue that it adds a bit of value. Not strong feelings, just a hunch.

except DataSetNotFoundError: # pragma: no cover
Copy link
Member

Choose a reason for hiding this comment

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

Wonder if this should be covered in tests, given that it does make a functional difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, i will write the tests now !

dataset_obj = None
dataset_obj = MemoryDataSet()

return dataset_obj

Expand Down