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

Add documentation for preview datasets in Kedro-viz #2773

Merged
merged 17 commits into from
Jul 11, 2023

Conversation

rashidakanchwala
Copy link
Contributor

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

Add documentation for preview datasets in Kedro-viz

Development notes

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

I didn't look at this in detail yet, commented on some whitespace and formatting issues

docs/source/visualisation/preview_datasets.md Outdated Show resolved Hide resolved
docs/source/visualisation/preview_datasets.md Outdated Show resolved Hide resolved
@astrojuanlu
Copy link
Member

xref kedro-org/kedro-viz#1650

rashidakanchwala and others added 3 commits July 6, 2023 14:40
Co-authored-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
Co-authored-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
Copy link
Contributor

@stichbury stichbury left a comment

Choose a reason for hiding this comment

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

LGTM. Just a few changes requested, mostly just small fixes to capitalisation but also please can you specify the version from which this is supported? I've made some suggestions for how to do this but you'll need to drop in the version to replace the two places I used whatever_it_is to represent the first version supporting the functionality.

stichbury and others added 5 commits July 6, 2023 17:30
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Copy link
Member

@tynandebold tynandebold left a comment

Choose a reason for hiding this comment

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

Looking good!

* `pandas.ExcelDataset`


To enable the preview of these datasets, you need to add the `preview_args` attribute to the `kedro-viz` configuration under the `metadata` section in the Data Catalog:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also state that nrow is number of rows to be previewed?

Copy link
Contributor

@stichbury stichbury left a comment

Choose a reason for hiding this comment

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

LGTM 🏆 Thank you!

@rashidakanchwala rashidakanchwala merged commit b1e3293 into main Jul 11, 2023
@rashidakanchwala rashidakanchwala deleted the docs/kedro-viz-preview-data branch July 11, 2023 17:18
jmnunezd pushed a commit to jmnunezd/kedro that referenced this pull request Jul 11, 2023
Add documentation for preview datasets in Kedro-viz

Signed-off-by: Jose <jmnunezd123@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants