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

Improve error message for %logstop #832

Closed
takluyver opened this issue Oct 1, 2011 · 4 comments
Closed

Improve error message for %logstop #832

takluyver opened this issue Oct 1, 2011 · 4 comments
Milestone

Comments

@takluyver
Copy link
Member

If %logstop is called when logging is not active, it throws a traceback with a fairly unhelpful AttributeError: 'NoneType' object has no attribute 'close'. We should catch this error and display a one-line message like "Logging hadn't been started."

@bernardpaulus
Copy link
Contributor

Hello!

By taking a peek in the current code (commit 088eb7d), it can solved by (in IPython/core/logger.py) :

def logstop(self):
    """Fully stop logging and close log file.

    In order to start logging again, a new logstart() call needs to be
    made, possibly (though not necessarily) with a new filename, mode and
    other options."""

    if self.logfile is not None:
        self.logfile.close()
        self.logfile = None
    self.log_active = False

and the diff

--- a/IPython/core/logger.py
+++ b/IPython/core/logger.py
@@ -206,8 +206,9 @@ which already exists. But you must first start the logging p
         made, possibly (though not necessarily) with a new filename, mode and
         other options."""

-        self.logfile.close()
-        self.logfile = None
+        if self.logfile is not None:
+            self.logfile.close()
+            self.logfile = None
         self.log_active = False

     # For backwards compatibility, in case anyone was using this.

I'm not familiar with git yet, so either you commit this directly, or you wait I read http://progit.org/book/ ;)

@takluyver
Copy link
Member Author

Thanks, Bernard. I think we should also display a brief error message when it's called from a magic command. That's best added to magic_logstop:

https://github.com/ipython/ipython/blob/master/IPython/core/magic.py#L1194

A brief rundown of using github:

Click the fork button to copy the repo to your own account. Then on your computer:

git clone https://github.com/bernardpaulus/ipython.git
cd ipython
git checkout -b logstop-error   # last bit is the branch name
# Change files as desired
git commit -a
git push origin logstop-error

Finally, back on github, find the branch you changed, and click the 'Pull request' button to submit the changes. We've got a more detailed explanation of the workflow in the docs: http://ipython.org/ipython-doc/stable/development/gitwash/index.html

@bernardpaulus
Copy link
Contributor

fixed in #834

I however put the print statement in logger.py since other functions inside it behave like that (switch_log() for instance https://github.com/ipython/ipython/blob/master/IPython/core/logger.py#L127 )

Thanks for your explanations!

@takluyver
Copy link
Member Author

Thanks - closing this issue.

markvoorhies pushed a commit to markvoorhies/ipython that referenced this issue Dec 3, 2013
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014
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

No branches or pull requests

2 participants