josch/img2pdf

Fork 10

Slightly simplify imgformat retrieval #201

Open

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?

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u imgformat:mara0004-imgformat

git checkout mara0004-imgformat

Merge

Merge the changes and update on Forgejo.

git checkout main

git merge --no-ff mara0004-imgformat

git checkout main

git merge --ff-only mara0004-imgformat

git checkout mara0004-imgformat

git rebase main

git checkout main

git merge --no-ff mara0004-imgformat

git checkout main

git merge --squash mara0004-imgformat

git checkout main

git merge --ff-only mara0004-imgformat

git checkout main

git merge mara0004-imgformat

git push origin main

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.