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

split functions from nlbin #244

Closed
wants to merge 3 commits into from

Conversation

ZhangXinNan
Copy link
Contributor

process1 in nlbin is too long, I split some functions from process1.

ocropus-nlbin Outdated
print_info("# image is empty: %s" % (fname))
return
image /= amax(image)
image = normalze(raw)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you !

@kba
Copy link
Collaborator

kba commented Sep 21, 2017

Makes sense to split up, easier to follow. Maybe the method names could be less generic (binary, normalize).

Need to test this, travis complains about undefined variables.

@ZhangXinNan
Copy link
Contributor Author

ZhangXinNan commented Sep 22, 2017

@kba Could you give a better name to 'normalize' ?
I update the binary function, please review it again .
Thank you !

@kba
Copy link
Collaborator

kba commented Sep 29, 2017

I renamed it normalize_raw_image, will merge once https://travis-ci.org/tmbdev/ocropy/builds/281275240 succeeds.

@kba
Copy link
Collaborator

kba commented Sep 29, 2017

Thanks, merged!

@kba kba closed this Sep 29, 2017
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.

3 participants