Add support for Hints in python-libzim #115
Conversation
Codecov Report
@@ Coverage Diff @@
## update-libzim #115 +/- ##
=================================================
- Coverage 97.94% 97.11% -0.83%
=================================================
Files 3 3
Lines 292 312 +20
=================================================
+ Hits 286 303 +17
- Misses 6 9 +3
Continue to review full report at Codecov.
|
|
@mgautierfr Does that fix openzim/libzim#590 as well? |
Not entirely. It allow sotoki to be updated to pass the correct hints to libzim (which will fix the issue) |
|
@mgautierfr hmmm... then it is not clear what is left to do to close the libzim bug. |
|
There is no libzim bug. Only a wrong usage of it. we must update its user (python-libzim and sotoki) to correctly use it. I've made half of the work here and @rgaudin seems to have made the other half as he has generated zim file with correct article listing (kiwix/kiwix-js#708 (comment)) What we need to do is stabilize this API, be more resilient to errors and maybe add some tests (This PR already update tests to pass hints, but do not test more) |
I'm on it |
There was a problem hiding this comment.
Fine with me. Used it with sotoki.
Added tests in 6528b53. @mgautierfr please check whether you are fine with this approach or not
This is a pretty simple implementation. Following commits will correctly test the feature and adapt a bit the API.
Failsafe approach to Hints usage: only fail on missing or non-dict-like values. Assume, `hints=` can be whatever and - only cherry-pick our expected Hint from there - cast any value to bool While this drifts a bit appart the zero-logic approach of the wrapper, it's a common pythonic approach for this kind of things. Main reason for this is that filtering and casting in Python allows exception to go up the stack while in-cpp casting does not and ends up ignored. Now testing: - Item without `get_hints()` - writer.Item without `get_hints()` impl. - `get_hints()` returning a non-dict value - `get_hints()` returning unexpected hints (filtered) - hints on redirection
|
(The push-force only rewrite my commit message to remove the WIP) |
|
@mgautierfr it seems the tests now fail as we are expecting a Runtime Error exception when adding content to a non-started Creator but we segfault instead. It was not the case earlier so that may have to do with your edits… Those tests are in It seems to not fail on github actions (failing ones are reader-related) but it does locally here. |
|
Apparently due to openzim/libzim#613 |
I think it wasn't failing on github because it used a nightly from the 22nd and the fix was merged-in on the 20th. makes sense |
Also Added native int as Hint test (filtered)
One may think that it would be valid to pass a dict with
an int as key that matches the Hints in libzim's item.h.
Those are filtered in pylibzim as not recognized as Hint objects.
Also Removing duplicate test
|
Rebased the commits ; I'm happy with the state of this. Failing tests are reader-based and regards updates to the test ZIM on libkiwix and would be adressed when fixing the reader I believe. –> merging. |
Fixes #93
This PR is not fully tested, no check that created zim are correct.
The error handling is not perfect. No handling of :