-
Notifications
You must be signed in to change notification settings - Fork 915
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
Enable adding new attributes to datasets #2440
Comments
I think the handling of the The only complication here is that If my_dataset:
....
metadata:
kedro-viz:
layer: raw We'll need to ensure they're still processed correctly and also make sure the implementation is backwards compatible with having layers outside the metadata key. |
After discussion with @merelcht and @AntonyMilneQB the implementation should be as follows:
These require further subtasks that I will define in separate issues, but I would like to get @idanov's and @rashidakanchwala's opinions here first. The order of tasks would look like this:
Questions to consider:
An alternative approach:Alternatively, we could just keep layer where it is and not process it if defined within the metadata. This doesn't address the DataCatalog processing an attribute that isn't used within Kedro, but allows for the introduction of the |
Questions to consider: Are there any oversights on moving the handling of the layer attribute to Viz? Are there any other users of the top-level layer attribute? |
Currently the layer logic resides in Kedro Framework in DataCatalog (https://github.com/kedro-org/kedro/blob/main/kedro/io/data_catalog.py#L270-L275). Kedro basically sends to kedro-viz - all the layers as a dict in the format below
On kedro-viz we simply read the above, and match the dataset name to the key it belongs to. (https://github.com/kedro-org/kedro-viz/blob/main/package/kedro_viz/data_access/repositories/catalog.py#L40-L47) I suppose now this will change so Kedro will only send a dict called metadata to Kedro-viz. And Kedro-viz will extract all the layer information and map it correctly. This is fine but I am not sure how to make it backward compatible in a clean way @merelcht @AntonyMilneQB ? |
Description
Implement the feature to allow users to add new attributes to datasets. Use the syntax decided in #2439
Context
Sub-task of #1076
To be done after: Decide on syntax to allow adding new attributes#2439
Open question
Where should the
metadata
(or other name) attribute go?AbstractDataSet
DataCatalog
like layers?The text was updated successfully, but these errors were encountered: