-
Notifications
You must be signed in to change notification settings - Fork 113
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
Include JSON dataset in the demo-project #1930
Conversation
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Hi @SajidAlamQB , I see that #1929 suggests to replace companies to json. I think it would be nice to introduce a new dataset rather than modifying existing datasets in the demo_project. Thank you |
I'm not opposed but I feel like adding another dataset would be a bit random. If we wanted to include it for this demo we would have to rework it to make it feel natural. |
Signed-off-by: huongg <[email protected]>
Signed-off-by: huongg <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @SajidAlamQB . Left a minor comment, otherwise looks fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @SajidAlamQB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi team,
Not sure if this is the best example of previewing JSONDatasets on Kedro-viz. It might be beneficial to provide more illustrative examples so that users can better understand how to use it. The current JSON is a direct conversion of a really long table. Ideally, we could slice it to show the top 5 or 6 rows and represent that in a JSON format.
Thanks!
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Description
Related to: #1929
Development notes
Replaced
companies.csv
withcompanies_json.json
and added a new node that converts the json into a csv. This should make the json pre-viewable in the demo.This is what it looks like with the new
companies_json.json
in the viewEdit:
After reviewing with Rashida we now have a new reporting node
get_top_shuttles_data
that takes the head of the input table and converts it into JSON. See below for results:Checklist
RELEASE.md
file