Use integer indexing on tuples in mimetypes#142542
Use integer indexing on tuples in mimetypes#142542Viicos wants to merge 1 commit intopython:mainfrom
mimetypes#142542Conversation
| encoding = None | ||
| ext = ext.lower() | ||
| types_map = self.types_map[True] | ||
| types_map = self.types_map[1] |
There was a problem hiding this comment.
The types_map is populated using boolean indexing: self.types_map[strict][ext] = type with strict having a default argument of True.
So I am not sure making only this change makes the code more readable.
There was a problem hiding this comment.
Right, the change as proposed is arguably detrimental to understandability, overall.
If we were writing this from scratch in modern Python we'd probably use an enumeration and use it as the allowed values for the 'strict' keyword...which would optimally be named something like 'strictness' so that, eg, MimeTypes(strictness=STRICT) would read sensibly. But making that kind of change at this point is probably more than we want to do, though you could propose something.
Maybe just a more verbose comment where those two tuples are defined?
There was a problem hiding this comment.
Ok this kind of explains why the True/False literals were used for indexing here. What about:
iff --git a/Lib/mimetypes.py b/Lib/mimetypes.py
index f3cdad66261..61afeca2b72 100644
--- a/Lib/mimetypes.py
+++ b/Lib/mimetypes.py
@@ -104,8 +104,9 @@ def add_type(self, type, ext, strict=True):
if not type:
return
- self.types_map[strict][ext] = type
- exts = self.types_map_inv[strict].setdefault(type, [])
+ map_index = 1 if strict else 0
+ self.types_map[map_index][ext] = type
+ exts = self.types_map_inv[map_index].setdefault(type, [])
if ext not in exts:
exts.append(ext)?
While I'm generally in favor of avoiding diff noise for cosmetic changes, I came across this when inspecting the logic of the module, and found it pretty cursed and took me a while to understand what the indexing with booleans was doing (in these cases,
types_mapandtypes_map_invare two-tuples).The change is trivial and doesn't require an issue or news entry.