mkepub
mkepub copied to clipboard
Add optional `ext` keyword argument to `set_cover()`
Previously, set_cover()
would rely on the Python standard module imghdr
to determine the type of the specified cover image file. Unfortunately, imghdr
does not support SVG, so set_cover()
could not handle that type (which is permitted by the EPUB spec).
Detecting SVG files programmatically is nontrivial. Instead, simply provide an option for the client to specify the file type with the new keyword argument.
This introduces a minimal addition to the public API of the module.
I have two reservations about this PR:
- The parameter name
ext
. It's not exactly an extension. But it's not exactly a MIME type, either. I'm not sure what a better name would be. - This PR introduces unit testing to the application. There weren't any examples to follow in the existing tests (only high-level integration tests), so I've used my personal preferences regarding mocking and using a class. I'm certainly open to other approaches.
Coverage increased (+0.02%) to 98.98% when pulling 2af5d92b99406e0e458dc91e665a9655d1979056 on elebow:set-cover-arg-extension into 7dd07d997c10e8d1b06b0cfbac233c52a6d6ca27 on anqxyr:master.
It's not exactly an extension. But it's not exactly a MIME type, either.
It is exactly an extension, since it's used as the extension in the filename for the cover. Although I think that using the full word extension
would be more clear for the public API instead of ext
.
This PR introduces unit testing to the application.
Not a fan of the unittest
lib or class-based tests. mkepub
already uses pytest
for testing, and it has fixtures that should work just fine with function-based tests. Will rewrite this a bit later before merging.
Changed the parameter name to extension
.
Changed the tests to use pytest
, but also pytest-mock
. I'm not sure how you feel about that plugin, but the general approach is about the same as it was before.
Regarding the test class, it's purely for organizational purposes in the test code (and allows, for example, pytest -k TestSetCover
). Here's a branch without it, if you prefer that: https://github.com/elebow/mkepub/commits/set-cover-arg-extension-no-test-class. I can update this PR you want.