Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor to_text() to return string instead of bytes #493

Merged
merged 1 commit into from
Mar 12, 2023

Conversation

rmilecki
Copy link
Collaborator

Function name "to_text" suggests it should return a text. Python uses
str type for storing texts. Having that function return bytes was
counter-intuitive.

This also simplifies code as most input methods deal with str type. With
this change there is no need to encode str into bytes and decode it
back.
There are actually only 2 input methods dealing with bytes: "pdftotext"
and "tesseract". Make them decode bytes into str before returning from
to_text().

Function name "to_text" suggests it should return a text. Python uses
str type for storing texts. Having that function return bytes was
counter-intuitive.

This also simplifies code as most input methods deal with str type. With
this change there is no need to encode str into bytes and decode it
back.
There are actually only 2 input methods dealing with bytes: "pdftotext"
and "tesseract". Make them decode bytes into str before returning from
to_text().
@rmilecki rmilecki requested a review from bosd March 12, 2023 10:28
@rmilecki
Copy link
Collaborator Author

@bosd: I thought about this small refactoring after your comment in the #481. Do you think it's OK?

Copy link
Collaborator

@bosd bosd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense!! 👍

@rmilecki rmilecki merged commit dcb19e5 into invoice-x:master Mar 12, 2023
@rmilecki rmilecki deleted the to_text-str branch May 11, 2023 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants