Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

fix loadpy duplicating newlines #1207

Merged
merged 2 commits into from

3 participants

@minrk
Owner

%loadpy script.py iterates through lines, but then does os.linesep.join(lines), thus duplicating all newlines. This uses splitlines(), so the content matches that fetched from a url - list of lines without endings.

closes #1204

@minrk
Owner

@fperez - This is due to the code you and I reviewed together on IRC. The answer seems pretty straightforward. An alternative implementation that avoids iterating twice through the lines is here. But I think the splitlines() implementation is easier to read, and iterating through a list should be a small factor compared to opening/reading a file.

@fperez
Owner

Problem. Running:

%loadpy http://matplotlib.sourceforge.net/mpl_examples/pylab_examples/integral_demo.py

produces

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
/home/fperez/tmp/ipython-notebooks/<ipython-input-1-af6e804f992c> in <module>()
----> 1 get_ipython().magic(u'loadpy http://matplotlib.sourceforge.net/mpl_examples/pylab_examples/integral_demo.py')

/home/fperez/usr/lib/python2.7/site-packages/IPython/core/interactiveshell.pyc in magic(self, arg_s, next_input)
   1995                 self._magic_locals = sys._getframe(1).f_locals
   1996             with self.builtin_trap:
-> 1997                 result = fn(magic_args)
   1998             # Ensure we're not keeping object references around:

   1999             self._magic_locals = {}

/home/fperez/usr/lib/python2.7/site-packages/IPython/core/magic.pyc in magic_loadpy(self, arg_s)
   2175             # logic, going with utf-8 is a simple solution likely to be right

   2176             # in most real-world cases.

-> 2177             with urllib2.urlopen(arg_s) as fileobj:
   2178                 linesource = fileobj.read().decode('utf-8', 'replace').splitlines()
   2179         else:

AttributeError: addinfourl instance has no attribute '__exit__'

It looks like the urllib objects are file-like but not proper context managers. That may be why we had the uglier manual handling of file closing by hand.

@minrk
Owner

ah, stupid and fixed. Thanks for the catch.

@fperez
Owner

Fix works, thanks. Merging now.

@fperez fperez merged commit a827fe6 into from
@fperez fperez referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@juliantaylor
Collaborator

you can make it one with contextlib.closing
http://docs.python.org/library/contextlib.html#contextlib.closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 24, 2011
  1. @minrk

    fix loadpy duplicating newlines

    minrk authored
Commits on Jan 6, 2012
  1. @minrk
This page is out of date. Refresh to see the latest.
Showing with 3 additions and 2 deletions.
  1. +3 −2 IPython/core/magic.py
View
5 IPython/core/magic.py
@@ -2173,12 +2173,13 @@ def magic_loadpy(self, arg_s):
# logic, going with utf-8 is a simple solution likely to be right
# in most real-world cases.
linesource = fileobj.read().decode('utf-8', 'replace').splitlines()
+ fileobj.close()
else:
- fileobj = linesource = open(arg_s)
+ with open(arg_s) as fileobj:
+ linesource = fileobj.read().splitlines()
# Strip out encoding declarations
lines = [l for l in linesource if not _encoding_declaration_re.match(l)]
- fileobj.close()
self.set_next_input(os.linesep.join(lines))
Something went wrong with that request. Please try again.