[Tutor] code improvement for beginner ?

[Danny Yoo]

> > The point of this restructuring is to allow you to add more image
> > types without too much pain, since there's no more hardcoded array
> > indexing against r1.  It also simplifies to calls to imgreg from:
> >
> >     if imgreg(r1[0],a) == 1:
> >         continue
> >     if imgreg(r1[1],a) == 1:
> >         continue
> >     imgreg(r1[2],a)
> >
> > to the simpler:
> >
> >     imgreg(a)
>
> Yes. Thats good.


Check your code: pageimgs() is calling imgreg three times.



> The problem with downloading the images is this:
>
> -------------
> http://images.nfl.com/images/globalnav-shadow-gray.gif
> Traceback (most recent call last):
>   File "/home/internet/bin/nflgrab.py", line 167, in ?
>     urllib.urlretrieve(img,img[f:])
>   File "/usr/lib/python2.3/urllib.py", line 83, in urlretrieve
>     return _urlopener.retrieve(url, filename, reporthook, data)
>   File "/usr/lib/python2.3/urllib.py", line 216, in retrieve
>     tfp = open(filename, 'wb')
> IOError: [Errno 13] Permission denied: '/globalnav-shadow-gray.gif'

One bug is that Python is trying to write those image files to the root
directory.  Your operating system's file system is saying that it won't
allow you to write files to that location.

urllib.urlretrieve saves those files with the path given in the second
parameter:

    urllib.urlretrieve(img, img[f:])
                            ^^^^^^^

You may want to change this so that it stores those files in a particular
directory.  Something like:

    urllib.urlretrieve(img, os.path.join("/tmp",
                                         img[f+1:]))

may work better.  The main idea is that you should explicitly control
where the files are being downloaded to.


Another bug: you will probably still run into problems because 'img' must
be a URL, and each 'img' is instead a line that contains a URL.  The
difference is between having a string like:

    http://python.org.

and:

    This line contains a url to the python web site: http://python.org.

and images, as far as I can tell, is storing a list of the lines, not the
image urls.  So you may want to make appropriate changes to imgreg() so
that it maintains a list of image urls.