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

[KED-2948] Fixed params as prefix issue for datasets #707

Merged

Conversation

rashidakanchwala
Copy link
Contributor

@rashidakanchwala rashidakanchwala commented Jan 19, 2022

Signed-off-by: Rashida Kanchwala [email protected]

Description

Fixes #621.

Development notes

'params:' and 'parameters' are reserved keywords. If anyone uses them in Kedro. The node will be considered as a parameter.

This issue has been fixed, so if you rename a dataset to param_<dataset_name>, it will still display as a dataset

Screenshot 2022-01-19 at 15 32 14

QA notes

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added new entries to the RELEASE.md file
  • Added tests to cover my changes

Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
def as_dict(self) -> Dict[str, ModularPipelineNode]:
"""Return the repository as a dictionary."""
return self.tree
"""`kedro_viz.data_access.repositories.modular_pipelines`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea, why github is not ignoring this since it's exact same !!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mysterious! Maybe pulling main in will fix it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad I'm not the only going crazy. I was looking through the blames and couldn't tell what changed!

Signed-off-by: Rashida Kanchwala <[email protected]>
Copy link
Contributor

@antonymilne antonymilne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix! Actually I just went to check how we do this in kedro and I think we've made the same mistake there 😅

Copy link
Member

@tynandebold tynandebold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one

@rashidakanchwala rashidakanchwala merged commit 0c68d30 into main Jan 21, 2022
@rashidakanchwala rashidakanchwala deleted the KED-2948/fix/params_prefix_for_non_param_datasets branch January 21, 2022 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants