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

Update README.rst warning: sacred vs ipython notebooks #103

Closed
wants to merge 1 commit into from

Conversation

matt32106
Copy link

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.889% when pulling b2ba98c on matt32106:master into bc9c0f3 on IDSIA:master.

@Qwlouse
Copy link
Collaborator

Qwlouse commented Apr 1, 2016

Hey matt32106,
thank you for adding that note! You are right it might be important to point that out.
I'd like to clarify though, that launching experiments from a notebook is possible. It is in fact a very common thing in my own workflow. I do that by importing the experiment from a file and calling ex.run(...) from the notebook.
What is not working is to define an experiment in a notebook.
If you could clarify that, I'd be happy to merge.

@Qwlouse
Copy link
Collaborator

Qwlouse commented Apr 1, 2016

Actually I've decided now to fix #100. Defining an experiment in a notebook is still not recommended, but it will work. This might remove the need for your note. What do you think?

@matt32106
Copy link
Author

Well, i have to say it will be easier for me, but the point you made in #100 was very valid.
Maybe you should add a non default option in the call of experiment so that user get a chance to reconsider (unsecure=true or something like that)

@Qwlouse
Copy link
Collaborator

Qwlouse commented Apr 14, 2016

That is a good point. Maybe I should change the behaviour to fail by default in interactive sessions, and have a unsafe=True or maybe better interactive=True option that converts this into a warning. I'll do that soon.

IMHO this would remove the requirement for an explicit warning in the README. What do you think?

@matt32106
Copy link
Author

I think that it should be somewhere in the documentation because people will have to make the link between ipython/jupyter and interactive = True/False, could be in the readme or module documentation but it should be mentioned at least once (for google :) ) somewhere

just my 2 cents :)
Matt

Date: Thu, 14 Apr 2016 02:21:10 -0700
From: git.m7.213feed42b.notifications#github.com@ob.0sg.net
To: reply@reply.github.com
CC: git.m7@xoxy.net
Subject: Re: [IDSIA/sacred] Update README.rst warning: sacred vs ipython
it: notifications@github.com exclusive) notebooks (#103)

That is a good point. Maybe I should change the behaviour to fail by default in interactive sessions, and have a unsafe=True or maybe better interactive=True option that converts this into a warning. I'll do that soon.

IMHO this would remove the requirement for an explicit warning in the README. What do you think?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@Qwlouse
Copy link
Collaborator

Qwlouse commented Aug 8, 2016

I've added a warning in the sphinx documentation and explained the interactive=True flag there. The error message and the API docs also contain the right pointers. I think this should be enough, and therefore close this issue.

@Qwlouse Qwlouse closed this Aug 8, 2016
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