[Biopython-dev] [Bug 2945] update_pdb: shutil.move needs to be indented; try block also?

[bugzilla-daemon at portal.open-bio.org]

http://bugzilla.open-bio.org/show_bug.cgi?id=2945





------- Comment #4 from TallPaulInJax at yahoo.com  2009-11-05 10:01 EST -------
I downloaded the updated PDBList.py from github. Unfortunately, testing fails
for several reasons:
1. The code you wrote includes an 'if' statement:
            if os.path.isfile(old_file) :
                try :
                    shutil.move(old_file, new_file)
                except :
                    warnings.warn("Could not move %s to obsolete folder" \
                                  % pdb_code, RuntimeWarning)
If the file does NOT exist, then no warning is issued as if the file HAD
existed and HAD been moved. This is a bug: there should be no if statement.
Simply try and move the file within a try/catch as I had written it without the
if statement:
          try :
               shutil.move(old_file, new_file)
          except :
               warnings.warn("Could not move %s to obsolete folder" \
                                  % pdb_code, RuntimeWarning)
That way no matter whether the file does not exist or cannot be moved, the
warning is issued. If you would like to trap the warnings separately, you
should write:
            if os.path.isfile(old_file) :
                try :
                    shutil.move(old_file, new_file)
                except :
                    warnings.warn("Could not move %s to obsolete folder" \
                                  % pdb_code, RuntimeWarning)
            else:
                warnings.warn("File %s not found to move to obsolete folder" \
                        % pdb_code, RuntimeWarning)

2. At least on Windows, if the subfolders of the obsolete folder do not exist,
a warning will be issued as well: python will create the 'obsolete' subolder
folder but will not create the subfolders under that. This will occur even if
the files DO exist and COULD be moved: a different kind of error. We just need
to create the subfolders, and warn if we can't.

3. Not a bug, but an enhancement: As a user, I'd rather see the whole new_file
name instead of the pdb_code.

To sum up, the below code will implement all those changes. Whether there are
other errors or not I have not checked. But I did check the above
warnings/errors with testing:

        for pdb_code in obsolete:
            if self.flat_tree:
                old_file = self.local_pdb + os.sep + 'pdb%s.ent'%(pdb_code)
                new_file = self.obsolete_pdb + os.sep + 'pdb%s.ent'%(pdb_code)
                new_path = self.obsolete_pdb  #<=====================
            else:
                old_file = self.local_pdb + os.sep + pdb_code[1:3] + os.sep +
'pdb%s.ent'%(pdb_code)
                new_file = self.obsolete_pdb + os.sep + pdb_code[1:3] + os.sep
+ 'pdb%s.ent'%(pdb_code)
                new_path = self.obsolete_pdb + os.sep + pdb_code[1:3]#<=====

            #If the old file doesn't exist, maybe someone else moved it
            #or deleted it already. Should we issue a warning?
            if os.path.isfile(old_file) :
                try :
                    os.makedirs(new_path) #<================
                    shutil.move(old_file, new_file)
                except :
                    warnings.warn("Could not move %s to obsolete folder" \
                                  % old_file, RuntimeWarning) #<====old_file
            else: #<===========
                warnings.warn("Could not find file %s to move to obsolete
folder" \
                              % old_file, RuntimeWarning) #<====old_file


-- 
Configure bugmail: http://bugzilla.open-bio.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.