AFPy/potodo
AFPy/potodo
11
3
Fork 7
Code Issues 3 Pull requests Projects Releases Packages Wiki Activity

Merge using polib #17

Merged
mdk merged 6 commits from maciek/potodo:merge-with-polib into main 2025-10-16 16:06:25 +02:00
Conversation 5 Commits 6 Files changed 2 +28 -17
maciek commented 2025-01-18 02:31:06 +01:00
Contributor
Copy link

Recently I found out that polib provides the msgmerge functionality. Therefore I propose simplifying the logic of merging and getting rid of GNU Gettext OS dependency.

This improves performance by 52%. (16.56 sec down to 7.93 sec on python-docs-pl / cpython build; 45% improvement considering msgmerge with --quiet flag.)

msgmerge
potodo --pot ../cpython/Doc/build/gettext  17.39s user 7.59s system 80% cpu 31.189 total
potodo --pot ../cpython/Doc/build/gettext  16.56s user 6.74s system 79% cpu 29.176 total
potodo --pot ../cpython/Doc/build/gettext  16.61s user 6.92s system 79% cpu 29.455 total

polib
potodo --pot ../cpython/Doc/build/gettext  7.93s user 0.38s system 88% cpu 9.422 total
potodo --pot ../cpython/Doc/build/gettext  8.40s user 0.44s system 88% cpu 10.036 total
potodo --pot ../cpython/Doc/build/gettext  7.53s user 0.36s system 88% cpu 8.900 total

msgmerge --quiet
potodo --pot ../cpython/Doc/build/gettext  14.32s user 5.43s system 79% cpu 24.815 total
potodo --pot ../cpython/Doc/build/gettext  14.25s user 5.78s system 80% cpu 24.898 total
potodo --pot ../cpython/Doc/build/gettext  14.51s user 5.48s system 79% cpu 25.110 total

7.93 / 16.56 (new/old)
0.4788647342995169: 52% time saved

Closes #19.

Recently I found out that polib provides [the msgmerge functionality](https://polib.readthedocs.io/en/latest/api.html#polib.POFile.merge). Therefore I propose simplifying the logic of merging and getting rid of GNU Gettext OS dependency. This improves performance by 52%. (16.56 sec down to 7.93 sec on python-docs-pl / cpython build; 45% improvement considering msgmerge with --quiet flag.) ``` msgmerge potodo --pot ../cpython/Doc/build/gettext 17.39s user 7.59s system 80% cpu 31.189 total potodo --pot ../cpython/Doc/build/gettext 16.56s user 6.74s system 79% cpu 29.176 total potodo --pot ../cpython/Doc/build/gettext 16.61s user 6.92s system 79% cpu 29.455 total polib potodo --pot ../cpython/Doc/build/gettext 7.93s user 0.38s system 88% cpu 9.422 total potodo --pot ../cpython/Doc/build/gettext 8.40s user 0.44s system 88% cpu 10.036 total potodo --pot ../cpython/Doc/build/gettext 7.53s user 0.36s system 88% cpu 8.900 total msgmerge --quiet potodo --pot ../cpython/Doc/build/gettext 14.32s user 5.43s system 79% cpu 24.815 total potodo --pot ../cpython/Doc/build/gettext 14.25s user 5.78s system 80% cpu 24.898 total potodo --pot ../cpython/Doc/build/gettext 14.51s user 5.48s system 79% cpu 25.110 total 7.93 / 16.56 (new/old) 0.4788647342995169: 52% time saved ``` Closes #19.
Remove GNU Gettext OS dependency
maciek changed title from Merge with polib to Merge using polib 2025-01-20 22:35:44 +01:00
mattwang44 reviewed 2025-02-04 06:01:16 +01:00
tests/test_merge.py Outdated
@ -22,3 +12,2 @@
Path(tmp_dir, "file1.po").read_text()
== """#: /un/chemin/idiot.rst:69
msgid "This is an updated dummy sentence."
== """#
mattwang44 commented 2025-02-04 05:59:47 +01:00
Contributor
Copy link

Why is there a pair of empty msgid and msgstr in the merging result?
Can we (or should we) get rid of it?

Why is there a pair of empty msgid and msgstr in the merging result? Can we (or should we) get rid of it?
maciek commented 2025-02-04 08:33:59 +01:00
Author
Contributor
Copy link

It's empty metadata entry, even though there's no metadata.

https://github.com/izimobil/polib/issues/160

It's empty metadata entry, even though there's no metadata. https://github.com/izimobil/polib/issues/160
mattwang44 commented 2025-02-04 10:01:13 +01:00
Contributor
Copy link

Maybe add a comment mentioning this polib issue?

Maybe add a comment mentioning this polib issue?
👍 1
maciek commented 2025-02-04 18:18:48 +01:00
Author
Contributor
Copy link

I've added the comment in the test. Thank you for the review!

I've added the comment in the test. Thank you for the review!
❤️ 1
maciek marked this conversation as resolved
mattwang44 approved these changes 2025-02-04 10:01:35 +01:00
rffontenelle commented 2025-02-05 19:16:45 +01:00
First-time contributor
Copy link

I tested and this fits perfectly for python-docs-pt-br, allowing to remove the gettext installation step. Thanks for this contribution.

I [tested](https://github.com/python/python-docs-pt-br/pull/259) and this fits perfectly for python-docs-pt-br, allowing to remove the gettext installation step. Thanks for this contribution.
maciek commented 2025-02-14 21:06:40 +01:00
Author
Contributor
Copy link

@rffontenelle wrote in #17 (comment):

I tested and this fits perfectly for python-docs-pt-br, allowing to remove the gettext installation step. Thanks for this contribution.

Thank you for the test!

@rffontenelle wrote in https://git.afpy.org/AFPy/potodo/pulls/17#issuecomment-4857: > I [tested](https://github.com/python/python-docs-pt-br/pull/259) and this fits perfectly for python-docs-pt-br, allowing to remove the gettext installation step. Thanks for this contribution. Thank you for the test!
mdk merged commit 2208de484a into main 2025-10-10 14:36:18 +02:00
mdk deleted branch merge-with-polib 2025-10-10 14:36:19 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
mattwang44
rffontenelle
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
4 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
AFPy/potodo!17
No description provided.