Skip to content
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

Prioritise user created catch-all dataset factory pattern over {default} from runners #3720

Closed
ankatiyar opened this issue Mar 18, 2024 · 0 comments · Fixed by #3859
Closed
Assignees
Labels
Issue: Bug Report 🐞 Bug that needs to be fixed

Comments

@ankatiyar
Copy link
Contributor

Description

From slack conversation with @noklam https://linen-slack.kedro.org/t/16713675/i-am-trying-to-create-a-catch-all-patterns-using-the-dataset#65f8bddf-cb66-4b35-8f5d-c0850669b907

The catch-all factory pattern added by the user in their catalog does not always override the default dataset set by the runner -
This works as expected ✅ -

"{catch_all}":
  type: pickle.PickleDataset
  filepath: data/06_models/{catch_all}.pickle

However, this pattern doesn't ❌ -

"{nok}":
  type: pickle.PickleDataset
  filepath: data/06_models/{nok}.pickle

Context

This behaviour was introduced in #3332 and was intentional and is documented, however, it is a little bit strange since for a user defined "catch-all" pattern to be used instead of the default dataset defined by the runner, which is {default} has to be ranked higher in terms of priority (alphabetically) to be picked up. That limits the names of the catch all patterns (should begin with a/b/c etc).

Expected Result

  • Should always use the catch all pattern defined by the user (if there is one) instead of the default provided by the runner
  • {default} can still be a protected catch-all pattern name that can only be used by the runner
  • We should add a regression test for this behaviour. The catch-all patterns behave as expected in 0.18.14 but not with 0.19.x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Bug Report 🐞 Bug that needs to be fixed
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants