[DataCatalog2.0]: KedroDataCatalog

[ElenaKhaustova]

Description

In this PR we add a new catalog KedroDataCatalog, which uses the DataCatalogConfigResolver and addresses:

• [DataCatalog]: Enhance _FrozenDatasets public API #3926
• [DataCatalog]: Iterate through datasets objects in the catalog #3916
• [DataCatalog]: Revise methods and properties names for DataCatalog #3931

Please see the suggested order of work in #3995 (comment) and comment below: #4151 (comment)

This PR is done on top of #4160 and relies on CatalogProtocol.

For the reviewers: this PR does not include unit-tests for KedroDataCatalog, they'll be added after the initial feedback.

Development notes

• We kept some old DataCatalog API to avoid multiple if/else branches depending on catalog type used in context, runner and session
• Removed _FrozenDatasets and access datasets as properties
• Added get dataset by name feature: dedicated function and access by key
• Added iterate over the datasets feature
• add_feed_dict() simplified and renamed to add_raw_data()
• Datasets' init was moved out from from_config() method

To test KedroDataCatalog modify your settings.py and run commands as usual:

# settings.py

from kedro.io import KedroDataCatalog
DATA_CATALOG_CLASS = KedroDataCatalog

kedro run
kedro catalog list/rank/resolve/create

Developer Certificate of Origin

We need all contributions to comply with the Developer Certificate of Origin (DCO). All commits must be signed off by including a Signed-off-by line in the commit message. See our wiki for guidance.

If your PR is blocked due to unsigned commits, then you must follow the instructions under "Rebase the branch" on the GitHub Checks page for your PR. This will retroactively add the sign-off to all unsigned commits and allow the DCO check to pass.

Checklist

• Read the contributing guidelines
• Signed off each commit with a Developer Certificate of Origin (DCO)
• 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
• Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

[noklam]

Based on the research findings, we decided to get rid of the FrozenDataset API. It was confusing for the users, and they almost never used it, preferring _get_dataset() instead. For now, I made getitem to return a deep copy for consistency with the datasets property. However, object assignment is not supported anyway, so users cannot override the specific dataset.

I guess it will be hard to prevent user assigned something that they are not gonna access later. We have similar issues previously where user try to assign something to catalog but find out it doesn't change anything because Kedro run always creates its own `catalog.

For deep-copy, would it works for datasets that are not "picklable" or dataset that comes with connection?

I left a comment in the other PR, I still found CatalogProtocol a bit weird but I am not sure what's convention did the Python community adopted, something like CatalogLike?

[ElenaKhaustova]

I guess it will be hard to prevent user assigned something that they are not gonna access later. We have similar issues previously where user try to assign something to catalog but find out it doesn't change anything because Kedro run always creates its own `catalog.

For deep-copy, would it works for datasets that are not "picklable" or dataset that comes with connection?

These two points totally make sense to me. However, _FrozenDataset didn't solve any of those problems as people were using _get_dataset(), which returns the reference to the dataset. Based on your second point, I returned to reference as well, so we do not require "pickable" datasets, though as far as I understand, ParallelRunner does.

[ElenaKhaustova]

I left a comment in the other PR, I still found CatalogProtocol a bit weird but I am not sure what's convention did the Python community adopted, something like CatalogLike?

As for naming convention, I see different examples: https://typing.readthedocs.io/en/latest/spec/protocol.html
But if looking at the existing ones they named like SupportsDataCatalog: https://docs.python.org/3/library/typing.html#protocols

So maybe the last is the better naming

[ElenaKhaustova]

Update for the reviewers

Since there is a proposal on the new interface (#4175), we removed some methods (__getitem__, __setitem__, __iter__) from the current implementation since they will most probably change during the implementation of the proposal.

If the proposal won't go through, they'll be added in a separate PR, so we do not block the current one.

[merelcht]

I've done a first review and added (mostly nit) comments. I'll do another review and have a proper look at the tests tomorrow.

[idanov]

I think it we should do it in here, since it will make sure we adhere to it. While it's optional, it's beneficial at no cost for us. Other implementers do not need to extend it though.

[idanov]

I'd put it directly into the data_catalog module.

[ElenaKhaustova]

Do you mean moving KedroDataCatalog class into data_catalog instead of creating a new file?

[idanov]

Shouldn't we resolve only if ds_name not in self._datasets? Or it's just to make the code a bit simpler?

[ElenaKhaustova]

Yeah, the point was to prevent you from complaining about nested ifs 😅 I moved it inside the condition now.

[idanov]

Could we rearrange this in such a way that we fail first and then continue with the successful path? Currently the flow is as follows:

• resolve the dataset pattern
• if not part of the materialised datasets, add from config
• get the dataset
• if the dataset does not exist (basically if it cannot be resolved nor existing), go with error scenario
• otherwise continue with non-error scenario

I think we can make the flow a bit less zig-zagy.

[ElenaKhaustova]

Well, we can only fail after we try to resolve. Otherwise, you get one more layer of if as the logic needs to go inside the if fail [] else [] scenario.

Now the logic is like this:

• if not part of the materialised datasets, resolve the dataset pattern
• if resolved, add from config
• get the dataset
• if the dataset does not exist (basically if it cannot be resolved nor exists), go with the error scenario
• otherwise, continue with a non-error scenario

[merelcht]

Some final small comments, but otherwise I'm very happy with how this looks! Great work @ElenaKhaustova ⭐ 🌟