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

Remove experiment tracking setup from viz starters #262

Merged
merged 3 commits into from
Jan 8, 2025

Conversation

ravi-kumar-pilla
Copy link
Contributor

Motivation and Context

Resolves kedro-org/kedro#4371

How has this been tested?

  • All CI tests should pass

Checklist

  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Assigned myself to the PR
  • Added tests to cover my changes

@ravi-kumar-pilla ravi-kumar-pilla marked this pull request as ready for review December 19, 2024 18:18
Copy link
Contributor

@ankatiyar ankatiyar left a comment

Choose a reason for hiding this comment

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

Briefly went through the code for these starters and I think there's some datasets in the catalog which refer to the tracking datasets so you might need to update the pipelines and the catalog as well.

metrics:
  type: tracking.MetricsDataset
  filepath: data/09_tracking/metrics.json

companies_columns:
  type: tracking.JSONDataset
  filepath: data/09_tracking/companies_columns.json

@ravi-kumar-pilla
Copy link
Contributor Author

Hi @ankatiyar and @DimedS , I am not sure if it is worth to keep the viz starters after removing ET. The difference I see are the plotly charts. I feel we can keep the viz starters and rename them to spaceflights-pandas and spaceflights-pyspark to get the plots. What do you think ?

Copy link
Contributor

@ankatiyar ankatiyar left a comment

Choose a reason for hiding this comment

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

This looks good to me. As for removing the starters entirely might require significant rework of the project creation flow and warrants some discussion. I left a comment on the main issue too kedro-org/kedro-viz#2202

Copy link
Member

@DimedS DimedS left a comment

Choose a reason for hiding this comment

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

Thanks, @ravi-kumar-pilla! I agree that we should consider highlighting something additional related to Viz in the starter, rather than removing it. Removing it would result in a significant change to the kedro new workflow.

@ravi-kumar-pilla ravi-kumar-pilla merged commit adc6eb8 into main Jan 8, 2025
27 checks passed
@ravi-kumar-pilla ravi-kumar-pilla deleted the chore/remove-et-setup branch January 8, 2025 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kedro-starters: remove the experiment tracking setup code
3 participants