Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplified code #5315

Merged
merged 3 commits into from Jun 30, 2021
Merged

Simplified code #5315

merged 3 commits into from Jun 30, 2021

Conversation

radarhere
Copy link
Member

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

src/PIL/Image.py Outdated
return (
self.convert("La")
self.convert(self.mode.replace("A", "a"))

This comment was marked as outdated.

@wiredfool
Copy link
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
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.

None yet

4 participants