Add Type Alias for datasets pattern and refactoring #2770
Conversation
Signed-off-by: Nok <nok.lam.chan@quantumblack.com>
noklam
changed the title
| @@ -30,6 +30,8 @@ | |||
| CREDENTIALS_KEY = "credentials" | |||
| WORDS_REGEX_PATTERN = re.compile(r"\W+") | |||
|
|
|||
| Patterns = dict[str, dict[str, Any]] | |||
There was a problem hiding this comment.
Is this an overkill? I found that we don't do this in kedro other than datasets which use DI_ and DO_ for data input/output. In this case I do think it has value as it helps a lot to read the code especially when we have a lot of dict of dict flowing around everywhere, I think this put some context.
@astrojuanlu WDYT about using type alias?
There was a problem hiding this comment.
I think this is a good use of a type alias! Not sure the lowercase dict will work for them (but if they do, fantastic - I sometimes get lost with Python typing)
| @@ -186,8 +188,8 @@ def __init__( # pylint: disable=too-many-arguments | |||
| self.layers = layers | |||
| # Keep a record of all patterns in the catalog. | |||
| # {dataset pattern name : dataset pattern body} | |||
| self._dataset_patterns = dict(dataset_patterns or {}) | |||
| self._load_versions = dict(load_versions or {}) | |||
| self._dataset_patterns = dataset_patterns or {} | |||
There was a problem hiding this comment.
This means that dataset_patterns don't get converted to dict anymore right? Hope there's not any weird corner case
There was a problem hiding this comment.
dataset_patterns is already a dict[str, dict[str, Any]], no?
There was a problem hiding this comment.
Indeed, I meant to say that I don't know how this function is used - if it's internal and we know that the inputs are dictionaries already, then all is fine 👍🏽
Co-authored-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
|
Closing it in favor of #2776 |
Description
Development notes
Checklist
RELEASE.mdfile