show Python mimetypes module some love - Code Review (original) (raw)

Can't Edit Can't Publish+Mail Start Review Created: 15 years, 9 months ago by Jacob Rus Modified: 9 years, 5 months ago Reviewers: Base URL: http://svn.python.org/view/\*checkout\*/python/trunk/ Visibility: Public.	Description From the python tracker page: http://bugs.python.org/issue6626 "This version (#4) switches to expressing the default types as a list of tuples instead of as a dict, so that we can include duplicate rows so that "reverse" type -> extension lookups will behave properly, once we start changing the actual content of the defaults. The types_map and common_types dictionaries (aliases to the singleton MimeTypes object's types_map property) have been left behaving as before for backwards compatibility. The tests still pass." Patch Set 1# Total comments: 9 Downloaded from: http://bugs.python.org/file14696/mimetypes4.diff Created: 15 years, 9 months ago Download[raw] [tar.bz2] Unified diffs Side-by-side diffs Delta from patch set Stats (+323 lines, -405 lines) Patch Lib/mimetypes.py View 6 chunks +322 lines, -404 lines 9 comments Download Lib/test/test_mimetypes.py View 1 chunk +1 line, -1 line 0 comments Download Messages Total messages: 3 Expand All Messages | Collapse All Messages Jacob Rus 15 years, 9 months ago (2009-08-12 04:51:39 UTC)#1 Sign in to reply to this message. Jacob Rus Add a bunch of comments. http://codereview.appspot.com/104091/diff/1/2 File Lib/mimetypes.py (left): http://codereview.appspot.com/104091/diff/1/2#oldcode222 Line 222: The way all ... 15 years, 9 months ago (2009-08-12 18:10:33 UTC)#2 Add a bunch of comments. http://codereview.appspot.com/104091/diff/1/2 File Lib/mimetypes.py (left): http://codereview.appspot.com/104091/diff/1/2#oldcode222 Line 222: The way all the functions below (I removed them) work is just horribly confusing. Admittedly somewhat less so than the days when they merely said, something to the effect of: def guess_type(url): init() return guess_type(url) But still pretty confusing, not to mention verbose. http://codereview.appspot.com/104091/diff/1/2 File Lib/mimetypes.py (right): http://codereview.appspot.com/104091/diff/1/2#newcode42 Line 42: "init", "suffix_map", "encodings_map", "types_map", "common_types", Actually, I'm thinking that I shouldn't add much if any to this, since I'd like to deprecate several of these functions/objects. http://codereview.appspot.com/104091/diff/1/2#newcode87 Line 87: I don't actually want to do a lazy init, I don't think: these are a handful of ~40kb files, and shouldn't take any noticeable amount of time or resources to parse. In the latest version on my disk, which I'll try to make a patch for and upload sometime, I merged this into __init__. http://codereview.appspot.com/104091/diff/1/2#newcode242 Line 242: } Previously, anyone who redefined suffix_map would permanently prevent other users from getting a 'clean' MimeTypes object, unless they called the private _default_mime_types function (as the tests did). This seemed pretty broken. Now, instead, making a new MimeTypes() object pulls from these private dicts, so that changing the public dicts only affects the module's MimeTypes singleton. http://codereview.appspot.com/104091/diff/1/2#newcode371 Line 371: ] These have been changed to a list of tuples so that declaring the same extension with two different types actually works as expected. (i.e. registers both) http://codereview.appspot.com/104091/diff/1/2#newcode383 Line 383: ] These should be removed altogether, because the only one that has any effect if a reasonably up-to-date Apache is installed on the system is the 'image/jpg' one. Frankly, I don't see the use case for having a separate 'lenient' registry, but I'm happy to discuss. http://codereview.appspot.com/104091/diff/1/2#newcode447 Line 447: strict = True It's probably unnecessary, but I tried to shorten the code throughout the module, wherever I could. Sign in to reply to this message. Jacob Rus a couple more comments. http://codereview.appspot.com/104091/diff/1/2 File Lib/mimetypes.py (right): http://codereview.appspot.com/104091/diff/1/2#newcode57 Line 57: ] I reordered these ... 15 years, 9 months ago (2009-08-12 18:15:25 UTC)#3 a couple more comments. http://codereview.appspot.com/104091/diff/1/2 File Lib/mimetypes.py (right): http://codereview.appspot.com/104091/diff/1/2#newcode57 Line 57: ] I reordered these so that more recent Apache versions will come later, so that we don't overwrite the more recent mappings with out-of-date ones. Personally, I'd prefer to remove the dependency on Apache, and just ship a big list of types inside the standard library, either inline in this module (as a list of tuples) or as a separate file. http://codereview.appspot.com/104091/diff/1/2#newcode88 Line 88: def add_types(self, type_pairs, strict=True): I think I'd actually rather call this method 'register', and not include one in the top level. If users want to add types, they should make their own MimeTypes object and not muck with the singleton. Sign in to reply to this message. Expand All Messages	Collapse All Messages

Issue 104091: show Python mimetypes module some love
Created 15 years, 9 months ago by Jacob Rus
Modified 9 years, 5 months ago
Reviewers:
Base URL: http://svn.python.org/view/\*checkout\*/python/trunk/
Comments: 9