josch/img2pdf

Fork 10

Slightly simplify imgformat retrieval #201

Closed

mara0004 wants to merge 2 commits from mara0004/img2pdf:imgformat into main

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.

mara0004 added 1 commit 2024-08-25 15:45:39 +00:00

Slightly simplify imgformat retrieval 132464c0a1

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

Update to getattr() 2b563c0982

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.

mara0004 referenced this pull request

2024-09-13 21:42:16 +00:00

Add support for JBIG2 (generic coding) #184

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.

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.

No reviewers

No labels

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.