josch/img2pdf
3
6
Fork 10
Code Issues24 Pull requests Projects Releases4 Wiki Activity

Slightly simplify imgformat retrieval #201

Closed
mara0004 wants to merge 2 commits from mara0004/img2pdf:imgformat into main
Conversation 6 Commits 2 Files changed 1 +1 -6
mara0004 commented 2024-08-25 15:45:37 +00:00
Contributor

No need for a loop here - we can access the enum like a dictionary.

No need for a loop here - we can access the enum like a dictionary.
mara0004 added 1 commit 2024-08-25 15:45:39 +00:00 
No need for a loop here - we can access the enum like a dictionary,
which should be more efficient.
mara0004 changed title from nit: slightly simplify imgformat retrieval to Slightly simplify imgformat retrieval 2024-08-25 18:37:51 +00:00
josch commented 2024-08-27 08:17:19 +00:00
Owner

No need for a loop here - we can access the enum like a dictionary.

Yes, if your python version is recent enough. Is there an objective reason to change this?

> No need for a loop here - we can access the enum like a dictionary. Yes, if your python version is recent enough. Is there an objective reason to change this?
mara0004 commented 2024-08-27 11:07:59 +00:00
Author
Contributor

Yes, if your python version is recent enough.

In the python docs, Enum __getitem__ has no versionadded note, so I don't think this is a recent addition? Moreover, I believe you're already using this syntax in other places of img2pdf, where dotted access would not work (e.g. Rotation["0"], Colorspace["CMYK;I"].

Is there an objective reason to change this?

Well, not that it matters much here, but the time complexity of a dict lookup should be O(1) whereas looping over the members is O(length). Also, the current loop seems to be lacking a break. So this is just a bit cleaner.

> Yes, if your python version is recent enough. In the python docs, Enum [`__getitem__`](https://docs.python.org/3/library/enum.html#enum.EnumType.__getitem__) has no versionadded note, so I don't think this is a recent addition? Moreover, I believe you're already using this syntax in other places of img2pdf, where dotted access would not work (e.g. `Rotation["0"]`, `Colorspace["CMYK;I"]`. > Is there an objective reason to change this? Well, not that it matters much here, but the time complexity of a dict lookup should be O(1) whereas looping over the members is O(length). Also, the current loop seems to be lacking a `break`. So this is just a bit cleaner.
mara0004 commented 2024-08-27 11:44:56 +00:00
Author
Contributor

Come to think of it, getattr(ImageFormat, imgdata.format, ImageFormat.other) would be even simpler than subscription in try/except.

Come to think of it, `getattr(ImageFormat, imgdata.format, ImageFormat.other)` would be even simpler than subscription in try/except.
mara0004 added 1 commit 2024-08-27 20:30:28 +00:00 
This now a one-liner, compared to 6 lines originally.
And it should be (algorithmically) faster.
mara0004 commented 2024-08-27 20:52:20 +00:00
Author
Contributor

I updated the code to getattr(...) as drafted above. It's a one-liner now.

I updated the code to `getattr(...)` as drafted above. It's a one-liner now.
josch commented 2024-09-15 05:48:01 +00:00
Owner

I updated the code to getattr(...) as drafted above. It's a one-liner now.

Thank you! Could you squash both commits into one?

> I updated the code to `getattr(...)` as drafted above. It's a one-liner now. Thank you! Could you squash both commits into one?
josch commented 2025-02-15 13:46:03 +00:00
Owner

Hi, I squashed both commits into one for you and pushed the result into main.

Hi, I squashed both commits into one for you and pushed the result into main.
josch closed this pull request 2025-02-15 13:46:22 +00:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
No reviewers
No labels
bug
duplicate
enhancement
forwarded
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: josch/img2pdf#201
No description provided.
Delete branch "mara0004/img2pdf:imgformat"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?