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 20 forks source link

Use of get_filename() is broken #71

Closed rgaudin closed 4 years ago

rgaudin commented 4 years ago

libzim allows the creation of article from a file using getFilename() which is replicated here in get_filename().

This is not directly usable here as getSize() assumes that user is implementing getData() (get_data() here) which renders the getFilename mode useless.

https://github.com/openzim/python-libzim/blob/488c6499a6fd9497308c9b0b1ccf8a663a02b2f0/libzim/lib.cxx#L177-L181

mgautierfr commented 4 years ago

That's why the wrapper should do the less :) We can simply remove the getSize implementation and ask user to implement it (potentially without getting the data itself). Or we move the implementation in the python world that would allow the user to overwrite it we implementing a article using get_filename.

My preference is for the first option but it introduces a API break.

rgaudin commented 4 years ago

I don't think breaking the API is too much trouble at this stage but having to implement get_size seems like an additional burden while 90% of the use cases are probably either getting the size of get_data or doing an fstat of the filename. But it's not terrible neither so I'd be OK with it.