Skip to content

Conversation

@Viicos
Copy link
Contributor

@Viicos Viicos commented Dec 10, 2025

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_map and types_map_inv are two-tuples).

The change is trivial and doesn't require an issue or news entry.

@Viicos Viicos requested a review from a team as a code owner December 10, 2025 21:39
@StanFromIreland StanFromIreland added skip issue skip news type-refactor Code refactoring (with no changes in behavior) labels Dec 10, 2025
encoding = None
ext = ext.lower()
types_map = self.types_map[True]
types_map = self.types_map[1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review skip issue skip news type-refactor Code refactoring (with no changes in behavior)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants