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

add the jupyter dependencies into starter #229

Merged
merged 2 commits into from
Aug 16, 2024
Merged

Conversation

noklam
Copy link
Contributor

@noklam noklam commented Aug 5, 2024

Motivation and Context

Part of #3643

This is equivalent to add ipylab into starter as it is the only dependency we have. We add kedro[jupyter] as this is maintained from the kedro side already.

@@ -2,6 +2,7 @@ ipython>=8.10
jupyterlab>=3.0
notebook
kedro~={{ cookiecutter.kedro_version }}
kedro[jupyter]
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to update the line above to kedro[jupyter]~={{ cookiecutter.kedro_version }} itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about this, but then decide to do this because Jupyter is usually not a mandatory requirement so people can remove it easily. (So i expect people can delete this line or move it to a dev-requirements.txt easily).

They are equivalent so I don't have strong opinion, which one do you prefer?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't feel strongly either tbh and that makes sense ^

@noklam noklam requested review from lrcouto and ankatiyar and removed request for ElenaKhaustova August 16, 2024 10:47
@noklam noklam merged commit dc60fc3 into main Aug 16, 2024
23 checks passed
@noklam noklam deleted the noklam/add-ipylab-starter branch August 16, 2024 11:19
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.

3 participants