python loop problem

mrbug · 2010-03-01 23:49:06

I could use a little bit of help with a loop in dvdtube if someone wouldn't mind taking a look at my code. It's available through svn at http://dvdtube.googlepages.com/svn or through the aur.

The get_and_convert function is giving me trouble starting at line 172. For some reason, I can't get it to download the correct number of videos in the proper order. I think that the problem lies in the loop structure, but since it's my code, I think that I'm too familiar with it. I'm sure that I'm overlooking some simple mistake...

Thanks in advance to anyone who can help! This one problem is holding me back from releasing version 0.5, which is basically going to be the last milestone before I consider the program to be basically feature-complete. The future milestones will all be enhancements.

keenerd · 2010-03-02 03:35:25

I can't run the code right now, so none of these directly fix your problem. But they will help with clarity and pythonic-ness, which might make a bug stand out more clearly. All this is just looking at get_and_convert specifically, but also apply to other points in the code.

The loop structure is a little odd. You should not be manually incrementing variables.

            vidnum = 0
            for download in downloads:
                # increment counter for filenames
                vidnum = vidnum + 1
                # do something with download

should be more like

            for vidnum, download in enumerate(downloads):
                # do something with download

This part seems really alarming:

        vidnum = 0
        while vidnum < linknum:
            for download in downloads:
                vidnum = vidnum + 1
                # do something with download

because this looks like you want to download the same videos over and over again if linknum is ever larger than len(downloads). If your purpose is to download just the first few of the downloads list, you would be better served by list slicing:

            for vidnum, download in enumerate(downloads[:linknum]):
                # do something with download

Another place where you should not be using explicit incrementing:

        counter = 0
        while counter < 100:
            counter = counter + 1
            # do something with counter

instead:

        for counter in range(100):
            # do something with counter

Note you also don't need the "counter = 0" line.

Now the part you asked for help with :-)
I'm having a little trouble following things, so just to make sure I understand it: You want to drop a set length of videos (a DVD's worth) into each directory. Use the same loop structures above to slim down this section as well. Pretty much every "var = var + 1" is not needed.

One bug jumps out at me:

        for download in downloads:
            if total > limit:
                # increment into the next directory
            else:
                # download one video

What happens to the current value of download when total > limit? It is thrown away and never downloaded. That entire else block should not be.

        for download in downloads:
            if total > limit:
                # increment into the next directory
            # download one video

mrbug · 2010-03-02 09:53:34

First, thanks for taking a look at my code -- I'm still a Python newbie, so there are a lot of things that I don't like yet like enumerate() and range()

My purpose is to download all videos, so I would have to do it the first way rather than slicing, but is good to know that it'll work that way!

The block with total > limit was really throwing me off... Looking at your comments, I believe that you are right about how the else block should be part of the "for downloads" block one indent to the left.

Thanks for all of the help, I'll make the changes today and give it a try! I really appreciate the second pair of eyes...

EDIT: I've made some major code cleanup changes (not done yet, though!), and it has fixed a couple little problems. The biggest fix is that it now will download the correct videos in the correct order the correct number of times!

I'm getting a LOT closer to 0.5, so thanks a lot for your help!

EDIT2: Everything's working now except for the cleanup() function!! I just need to figure out the logic to make it go through the directories and rename the final files.

EDIT3: Thanks yet again... By making it more "Pythonic," I was able to fix cleanup() and release version 0.4.9, which is a release candidate for 0.5!

Last edited by mrbug (2010-03-03 21:15:56)

Arch Linux

#1 2010-03-01 23:49:06

python loop problem

#2 2010-03-02 03:35:25

Re: python loop problem

#3 2010-03-02 09:53:34

Re: python loop problem

Board footer