Move tarfile.filemode() into stat module
http://hg.python.org/cpython/file/9d9495fabeb9/Lib/tarfile.py#l304 I discovered this undocumented function by accident different years ago and reused it a couple of times since then. I think that leaving it hidden inside tarfile module is unfortunate. What about moving it into stat module and document it? Regards, --- Giampaolo http://code.google.com/p/pyftpdlib/ http://code.google.com/p/psutil/ http://code.google.com/p/pysendfile/
2012/5/12 Antoine Pitrou <solipsis@pitrou.net>:
Hmm... right. It's controversial. On one hand stat module looks better because the "mode" concept is scattered all over the place, on the other hand this is a perfect example of file-related "utility" function. Let's wait in order to collect some bikeshedding then. =) --- Giampaolo http://code.google.com/p/pyftpdlib/ http://code.google.com/p/psutil/ http://code.google.com/p/pysendfile/
On 5/12/2012 10:41 AM, Antoine Pitrou wrote:
I think I would more likely look in stat, and as noted below, the constants used for the table used in the function are already in stat.py. I checked, and # Bits used in the mode field, values in octal. #--------------------------------------------------------- S_IFLNK = 0o120000 # symbolic link ... are only used in filemode_table = ( ((S_IFLNK, "l"), ... which is only used in def filemode(mode): ... So all three can be cleanly extracted into another module. However 1) the bit definitions themselves should just be deleted as they *duplicate* those in stat.py. The S_Ixxx names are the same, the other names are variations of the other stat.S_Ixxxx names. So filemode_table (with '_' added?) could/should be re-written in stat.py to use the public, documented constants already defined there. However 2) stat.py lacks the nice comments explaining the constants in the file itself, so I *would* copy the comments to the appropriate lines. There only seems to be one use of the function in tarfile.py: Line 1998: print(filemode(tarinfo.mode), end=' ') All the other uses of 'filemode' are as a local name inside the open method, derived from its mode parameter: filemode, comptype = mode.split(":", 1) +1 on moving the table (probably with private name, and using the existing, documented stat S_Ixxxx constants) and function (public) to stat.py. -- Terry Jan Reedy
2012/5/12 Terry Reedy <tjreedy@udel.edu>:
Agreed then. I'm going to submit a patch soon. --- Giampaolo http://code.google.com/p/pyftpdlib/ http://code.google.com/p/psutil/ http://code.google.com/p/pysendfile/
2012/5/12 Terry Reedy <tjreedy@udel.edu>:
However 2) stat.py lacks the nice comments explaining the constants in the file itself, so I *would* copy the comments to the appropriate lines.
+1 If no one is opposed I'll do that tomorrow.
Add me terry.reedy as nosy and I will help review by checking the S_Ixxx substitutions.
Thanks, will do. --- Giampaolo http://code.google.com/p/pyftpdlib/ http://code.google.com/p/psutil/ http://code.google.com/p/pysendfile/
2012/5/12 Antoine Pitrou <solipsis@pitrou.net>:
Hmm... right. It's controversial. On one hand stat module looks better because the "mode" concept is scattered all over the place, on the other hand this is a perfect example of file-related "utility" function. Let's wait in order to collect some bikeshedding then. =) --- Giampaolo http://code.google.com/p/pyftpdlib/ http://code.google.com/p/psutil/ http://code.google.com/p/pysendfile/
On 5/12/2012 10:41 AM, Antoine Pitrou wrote:
I think I would more likely look in stat, and as noted below, the constants used for the table used in the function are already in stat.py. I checked, and # Bits used in the mode field, values in octal. #--------------------------------------------------------- S_IFLNK = 0o120000 # symbolic link ... are only used in filemode_table = ( ((S_IFLNK, "l"), ... which is only used in def filemode(mode): ... So all three can be cleanly extracted into another module. However 1) the bit definitions themselves should just be deleted as they *duplicate* those in stat.py. The S_Ixxx names are the same, the other names are variations of the other stat.S_Ixxxx names. So filemode_table (with '_' added?) could/should be re-written in stat.py to use the public, documented constants already defined there. However 2) stat.py lacks the nice comments explaining the constants in the file itself, so I *would* copy the comments to the appropriate lines. There only seems to be one use of the function in tarfile.py: Line 1998: print(filemode(tarinfo.mode), end=' ') All the other uses of 'filemode' are as a local name inside the open method, derived from its mode parameter: filemode, comptype = mode.split(":", 1) +1 on moving the table (probably with private name, and using the existing, documented stat S_Ixxxx constants) and function (public) to stat.py. -- Terry Jan Reedy
2012/5/12 Terry Reedy <tjreedy@udel.edu>:
Agreed then. I'm going to submit a patch soon. --- Giampaolo http://code.google.com/p/pyftpdlib/ http://code.google.com/p/psutil/ http://code.google.com/p/pysendfile/
2012/5/12 Terry Reedy <tjreedy@udel.edu>:
However 2) stat.py lacks the nice comments explaining the constants in the file itself, so I *would* copy the comments to the appropriate lines.
+1 If no one is opposed I'll do that tomorrow.
Add me terry.reedy as nosy and I will help review by checking the S_Ixxx substitutions.
Thanks, will do. --- Giampaolo http://code.google.com/p/pyftpdlib/ http://code.google.com/p/psutil/ http://code.google.com/p/pysendfile/
participants (3)
-
Antoine Pitrou -
Giampaolo Rodolà -
Terry Reedy