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 SensiML notebook, based on TensorFlow notebook #1234

Closed
wants to merge 2 commits into from

Conversation

jublin
Copy link

@jublin jublin commented Feb 8, 2021

No description provided.

Justin Moore added 2 commits February 8, 2021 08:51
Update documentation for specifics, remove unused code.

rewording
@romainx
Copy link
Collaborator

romainx commented Feb 18, 2021

Hello @jublin ,

Sorry for the late answer and thanks for this PR. However we are sorry but we are not extending the core stack with new images, that takes instead the community images path. Here is a recent comment explaining this process.

We've been requesting that all new image definitions take the community images path, primarily because this repository is unwieldy and we're already finding it difficult to maintain the images that already exist here. It sounds like you already have a nice project setup for building the images. Would you be open to submitting a pull request linking to it from the documentation instead of this PR?

Another solution is to integrate from this PR only the "recipe" part.
So if you agree, we could merge the recipe you wrote in recipes.md explaining how to build a sensiml-notebook image from the tensorflow-notebook image. If it's Ok for you, you can modify your PR to include only this description.

Please keep us informed of what you want to do between both solutions.

Best.

Copy link
Collaborator

@romainx romainx left a comment

Choose a reason for hiding this comment

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

Hello @jublin,

Do you want to change your PR to keep only the recipes.md and I will merge it? In the other case I will close it for the reasons mentioned in my previous comment.

Many thanks

@romainx romainx marked this pull request as draft May 4, 2021 05:41
Copy link
Member

@mathbunnyru mathbunnyru left a comment

Choose a reason for hiding this comment

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

@jublin are you still interested in merging this?

Please, take a look at the comment above and after the change proposed this will be a good recipe for the community.

If not, please, close the PR.

jupyter nbextension enable --py --sys-prefix qgrid && \
fix-permissions "${CONDA_DIR}" && \
fix-permissions "/home/${NB_USER}"

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

@consideRatio
Copy link
Collaborator

Given previous comments, I'll go ahead and close this PR at this point.

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.

4 participants