Skip to content

Simplified code#5315

Merged
radarhere merged 3 commits intopython-pillow:masterfrom
radarhere:simplified
Jun 30, 2021
Merged

Simplified code#5315
radarhere merged 3 commits intopython-pillow:masterfrom
radarhere:simplified

Conversation

@radarhere
Copy link
Copy Markdown
Member

I think this change simplifies the code. Feel free to disagree.

src/PIL/Image.py Outdated
if self.mode in ("LA", "RGBA"):
return (
self.convert("La")
self.convert(self.mode.replace("A", "a"))

This comment was marked as outdated.

@wiredfool
Copy link
Copy Markdown
Member

I don't like this, while it does remove a duplicated bit of code, it obscures what's happening.

In my opinion,

  • Modes are essentially enums that happen to be strings. Doing string.replace is kind of weird on an enum.
  • It reduces greppability. It's nice to be able to grep the code and find everywhere La is referenced.

If really want to do something to remove the duplicate code, perhaps a dict with a mapping of {'LA': 'La', 'RGBA':'RGBa'} would be appropriate.

@radarhere
Copy link
Copy Markdown
Member Author

I've pushed a commit to use the mapping option, and in two other locations as well.

@radarhere radarhere merged commit cab9179 into python-pillow:master Jun 30, 2021
@radarhere radarhere deleted the simplified branch June 30, 2021 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants