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

Fix MemoryDataSet not displaying on metadata panel #1113

Merged
merged 12 commits into from
Oct 3, 2022
+22 −8
Merged

Fix MemoryDataSet not displaying on metadata panel #1113

Show file tree
Hide file tree
Changes from 5 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
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
5 changes: 4 additions & 1 deletion 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 @@ -3,7 +3,7 @@
# pylint: disable=missing-class-docstring,missing-function-docstring,protected-access
from typing import Dict, Optional

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 @@ -57,6 +57,9 @@ def get_dataset(self, dataset_name: str) -> Optional[AbstractDataSet]:
dataset_obj = self._catalog._get_dataset(dataset_name)
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
# if dataset has no catalog entry, it is a MemoryDataSet
if not dataset_obj:
dataset_obj = MemoryDataSet()
Copy link
Contributor

@antonymilne antonymilne Oct 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Is this any different from the below? Can dataset_obj ever evaluate to false if the exception doesn't get thrown?

Suggested change
dataset_obj = None
# if dataset has no catalog entry, it is a MemoryDataSet
if not dataset_obj:
dataset_obj = MemoryDataSet()
dataset_obj = MemoryDataSet()

Copy link
Contributor Author

@rashidakanchwala rashidakanchwala Oct 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

it work's if you just add it in the exception. the reason why it flickered was because filepath is none for MemoryDataSets and it is a required field in DataNodeMetaDataResponseAPI which now I have made Optional.

I will make the above change :)

Copy link
Member

Choose a reason for hiding this comment

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

I'm happy with this. Yeah, I just didn't understand/get to finding what was happening with the validation error, but this solves it.


return dataset_obj

Expand Down