search

openzim / python-libzim

Libzim binding for Python: read/write ZIM files in Python
https://pypi.org/project/libzim/
GNU General Public License v3.0
62 stars 22 forks source link

Wrap indexData #149

Closed mgautierfr closed 2 years ago

mgautierfr commented 2 years ago

Fixes #92

codecov[bot] commented 2 years ago

Codecov Report

Base: 96.88% // Head: 92.87% // Decreases project coverage by -4.00% :warning:

Coverage data is based on head (a649a61) compared to base (66dac18). Patch coverage: 58.33% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #149 +/- ## ========================================== - Coverage 96.88% 92.87% -4.01% ========================================== Files 1 1 Lines 417 463 +46 ========================================== + Hits 404 430 +26 - Misses 13 33 +20 ``` | [Impacted Files](https://codecov.io/gh/openzim/python-libzim/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openzim) | Coverage Δ | | |---|---|---| | [libzim/libzim.pyx](https://codecov.io/gh/openzim/python-libzim/pull/149/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openzim#diff-bGliemltL2xpYnppbS5weXg=) | `92.87% <58.33%> (-4.01%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openzim). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openzim)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

mgautierfr commented 2 years ago

BTW, I'm not sure of the best casing. get_indexdata, get_indexData, get_index_data ? get_wordcount, get_word_count, get_wordCount, get_wordscount ?

rgaudin commented 2 years ago

BTW, I'm not sure of the best casing. get_indexdata, get_indexData, get_index_data ? get_wordcount, get_word_count, get_wordCount, get_wordscount ?

Ah I was wondering also what would be the most self-explanatory.

I'd suggest: get_indexdata, get_wordcount, get_geoposition and so on but any of them makes sense so you should go with what you feel is more appropriate.

mgautierfr commented 2 years ago

I'd suggest: get_indexdata, get_wordcount, get_geoposition and so on but any of them makes sense so you should go with what you feel is more appropriate.

It is what I've used. So we are good. I let you merge if you are ok with the PR

kelson42 commented 2 years ago

Code coverage has dropped significantly. A bit of automated testing might be welcome.

stale[bot] commented 2 years ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

kelson42 commented 2 years ago

@mgautierfr Please finish this Pr.

mgautierfr commented 2 years ago

Looks good but any exception in the IndexData methods ends up crashing the process. All the _cy_call_fct functions do enter the except block and set the trackeback in error[0] but for some reason it crashes instead of forwarding it up

This is more related to #42 than wrapping IndexData. I have a working version which need changes in python-libzim and libzim itself. I will create PRs soon.

rgaudin commented 2 years ago

As discussed, merging this despite the error handling issue and coverage.