Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Documentation for Preview Dataset #1757
Documentation for Preview Dataset #1757
Changes from 11 commits
691c7ba
70fa78f
1b692f8
9e93ca8
f57b1b6
2628396
93b79ae
6e83fc5
fb2d6e7
8c80933
2a9c923
919ab73
b57b3d7
5d4963f
401434e
c8e8f84
e296352
ff7b87b
22809be
fdd9613
9f67c74
1d89fed
ed42972
cf7a2cb
ebf6f30
074c92b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Any chance to add a link to such file?
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.
is the github link fine ? - https://github.com/kedro-org/kedro-plugins/blob/main/kedro-datasets/kedro_datasets/_typing.py
I can't seem to find docs source code link for _typing.py
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.
Yeah this is not documented, so a link to the source code is fine for now.
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.
Can we show an example for each type? This help users to link what are they expecting to see in the UI.
https://clear.ml/docs/latest/docs/guides/reporting/plotly_reporting
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.
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.
Can we maybe add an example that works, to give users a bit more guidance on how they should realistically implement a
preview()
method?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.
I will need some help on this as I can't think of a realistic CustomDataset that is not a part of kedro-datasets. @astrojuanlu -- do you have some ideas?
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.
When I suggested adding a working example, I didn't necessarily mean anything complex, just in this case some code that would produce a working
TablePreview
and demonstrates hownrows
,ncolumns
andfilters
would be used.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.
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.
FYI - this entire section has now moved to
Preview Tabular Data on Kedro-viz
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.
This content in this section is not new and it is just moved to a new page. Earlier it was a part of the 'Preview Datasets' page
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.
Probably capitalise Spaceflights?
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.
I noticed it's mostly lower-case elsewhere in the docs so I will leave it as is.
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.
This was asked in the Slack once, I think we should make it obvious that the
preview_args
is the argument that get pass into thepreview
function directly, and user can have arbitary arguments.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.
So I think for pandas datasets .. it is specific to
nrows
as we wrote thepreview()
func.But I have updated the custom dataset docs to include arguments. Thanks for highlighting this @noklam