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
Add docs for
KedroDataCatalog
#4249Add docs for
KedroDataCatalog
#4249Changes from 10 commits
241fb59
c38f3c0
f7016f5
a3442d5
33e314a
44d0207
50f236d
ab57346
527bc3f
3675539
14913a0
417c7b7
40c539b
d9c0b4b
9d153b1
ac83a8c
164d16e
981e710
9b8a61a
9870aed
bf3eb68
941caaf
06db04a
a5affbf
a370536
af58fae
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 50 in docs/source/data/index.md
GitHub Actions / vale
[vale] docs/source/data/index.md#L50
Raw output
Check warning on line 50 in docs/source/data/index.md
GitHub Actions / vale
[vale] docs/source/data/index.md#L50
Raw output
Check warning on line 70 in docs/source/data/index.md
GitHub Actions / vale
[vale] docs/source/data/index.md#L70
Raw output
Check warning on line 2 in docs/source/data/kedro_data_catalog.md
GitHub Actions / vale
[vale] docs/source/data/kedro_data_catalog.md#L2
Raw output
Check warning on line 16 in docs/source/data/kedro_data_catalog.md
GitHub Actions / vale
[vale] docs/source/data/kedro_data_catalog.md#L16
Raw output
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.
Unrelated to docs. I find this not very intuitive, is there a symmetry between the get/setter? i.e.
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.
The setter acts similar to the
add_feed_dict()
, so it allows you to add either datasets or raw data. There's a note below explaining that.When you add raw data, it is automatically wrapped in a `MemoryDataset` under the hood.
Check warning on line 39 in docs/source/data/kedro_data_catalog.md
GitHub Actions / vale
[vale] docs/source/data/kedro_data_catalog.md#L39
Raw output
Check warning on line 43 in docs/source/data/kedro_data_catalog.md
GitHub Actions / vale
[vale] docs/source/data/kedro_data_catalog.md#L43
Raw output
Check warning on line 78 in docs/source/data/kedro_data_catalog.md
GitHub Actions / vale
[vale] docs/source/data/kedro_data_catalog.md#L78
Raw output
Check warning on line 78 in docs/source/data/kedro_data_catalog.md
GitHub Actions / vale
[vale] docs/source/data/kedro_data_catalog.md#L78
Raw output
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 find this is not very useful since this is more about educating users printing is not needed in a terminal? The most common use case is Notebook, but I don't think we need to explains how this work in IPython/Notebook etc.
I would like to see what
catalog
andcatalog["reviews"]
print differently instead (with an example), maybe something with this format: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've added an example of the output instead
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.
Not strictly related to docs, do we expect Config Resolver as something that user will interact directly? I expect it's more of a refactoring and not a very user facing component (like
KedroContext
).Would it be bad if we wrap it under
KedroDataCatalog
? i.e.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.
We do not expect users to interact with it much, though they can. It's mostly needed for catalog CLI commands and maybe some other advanced usage. But we do not want to extend catalog API with these public methods as then it will have to be part of the
Protocol
.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.
Why is
del
not supported, is it an implementation issue? What's the correct way to remove a dataset from catalog?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.
It's not an issue, it's done intentionally as we don't want datasets to be removed by users manually. Of course, one can still remove it from the private
_datasets
dictionary but we do not provide an API for that.Check warning on line 100 in docs/source/data/kedro_data_catalog.md
GitHub Actions / vale
[vale] docs/source/data/kedro_data_catalog.md#L100
Raw output
Check warning on line 100 in docs/source/data/kedro_data_catalog.md
GitHub Actions / vale
[vale] docs/source/data/kedro_data_catalog.md#L100
Raw output